From: Alexander Sverdlin <alexander.sverdlin@nokia.com>
To: "linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>
Cc: Vlad Yasevich <vyasevich@gmail.com>,
Neil Horman <nhorman@tuxdriver.com>,
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
Petr Malat <oss@malat.biz>
Subject: Race of sctp_assoc_control_transport() against sctp_assoc_rm_peer() ?
Date: Tue, 8 Dec 2020 21:59:51 +0100 [thread overview]
Message-ID: <86d2a00b-7fba-a539-b1bf-5bacf0443542@nokia.com> (raw)
Hello!
I'm trying to understand the crash we experience with Linux v4.19
(sorry for the ancient codebase, but actually the affected code is largely
unchanged up to now).
This is hard but reproducible:
general protection fault: 0000 [#1] PREEMPT SMP NOPTI
CPU: 1 PID: 0 Comm: swapper/1 Tainted: G O 4.19.155-g...
...
RIP: 0010:sctp_assoc_control_transport+0x1db/0x290 [sctp]
Code: 01 00 00 00 00 00 00 bd 01 00 00 00 31 c0 48 89 e7 b9 10 00 00 00 f3 48 ab 48 8b 86 a8 00 00 00 48 89 e7 48 81 c6 88 00 00 00 <48> 63 90 bc 00 00 00 e8 29 61 2b e1 31 d2 41 b9 20 00 48 00 41 89
RSP: 0018:ffff88846fc43ba8 EFLAGS: 00010286
RAX: 7070682e72656d69 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000001 RSI: ffff8882984b3888 RDI: ffff88846fc43ba8
RBP: 0000000000000001 R08: 0000000000022e80 R09: ffff88846fc43cd8
R10: ffffffffa0562cf0 R11: 0000000000000007 R12: 0000000000000000
R13: ffff88846fc43cd8 R14: ffff8881bd588000 R15: ffff88846fc43e18
FS: 0000000000000000(0000) GS:ffff88846fc40000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000024b3160 CR3: 000000000320a006 CR4: 00000000003607e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<IRQ>
sctp_do_8_2_transport_strike.isra.10+0xd3/0x1a0 [sctp]
? sctp_oname+0x20/0x20 [sctp]
sctp_do_sm+0x15fb/0x1d00 [sctp]
? try_to_wake_up+0x226/0x4b0
? __update_load_avg_se+0x219/0x2c0
? enqueue_entity+0xc4/0x850
? enqueue_entity+0x17f/0x850
? enqueue_task_fair+0xe5/0x950
? __update_load_avg_cfs_rq+0x1e2/0x280
? resched_curr+0x20/0xd0
? check_preempt_curr+0x4e/0x90
? ttwu_do_wakeup.isra.21+0x19/0x170
sctp_generate_timeout_event+0xa8/0xf0 [sctp]
? sctp_generate_t4_rto_event+0x20/0x20 [sctp]
? sctp_generate_t4_rto_event+0x20/0x20 [sctp]
call_timer_fn+0x32/0x170
expire_timers+0x9d/0x110
run_timer_softirq+0x8a/0x160
? __hrtimer_run_queues+0x156/0x2e0
__do_softirq+0xaf/0x33e
irq_exit+0xbf/0xe0
smp_apic_timer_interrupt+0x7d/0x170
apic_timer_interrupt+0xf/0x20
</IRQ>
The exploding code in sctp_assoc_control_transport() is:
/* Generate and send a SCTP_PEER_ADDR_CHANGE notification
* to the user.
*/
if (ulp_notify) {
memset(&addr, 0, sizeof(struct sockaddr_storage));
memcpy(&addr, &transport->ipaddr,
transport->af_specific->sockaddr_len);
where "transport" pointer seems to be freed and re-used, so the dereference
to obtain "af_specific" leads to the dump above. This memset/memcpy pair
has been factored out into sctp_ulpevent_notify_peer_addr_change(), but
this most probably doesn't solve the problem.
According to the stack above, the race seems to be between this code:
enum sctp_disposition sctp_sf_t4_timer_expire(
struct net *net,
const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
const union sctp_subtype type,
void *arg,
struct sctp_cmd_seq *commands)
{
struct sctp_chunk *chunk = asoc->addip_last_asconf;
struct sctp_transport *transport = chunk->transport;
SCTP_INC_STATS(net, SCTP_MIB_T4_RTO_EXPIREDS);
/* ADDIP 4.1 B1) Increment the error counters and perform path failure
* detection on the appropriate destination address as defined in
* RFC2960 [5] section 8.1 and 8.2.
*/
if (transport)
sctp_add_cmd_sf(commands, SCTP_CMD_STRIKE,
SCTP_TRANSPORT(transport));
and
void sctp_assoc_rm_peer(struct sctp_association *asoc,
struct sctp_transport *peer)
{
...
/* If we remove the transport an ASCONF was last sent to, set it to
* NULL.
*/
if (asoc->addip_last_asconf &&
asoc->addip_last_asconf->transport == peer)
asoc->addip_last_asconf->transport = NULL;
...
asoc->peer.transport_count--;
sctp_transport_free(peer);
}
So instead of doing sctp_transport_hold() on the addip_last_asconf->transport,
the code relies on the NULL assignment to be propagated.
As I do not see any memory barrier or lock here, I have several questions:
- Is it possible that sctp_assoc_control_transport() runs in a timer handler
in parallel to sctp_assoc_rm_peer() running on a different core?
In this case there would be no guarantee, that NULL assignment will reach
another core.
- What was the designed guarantee, that sctp_assoc_control_transport() will see
the change to asoc->addip_last_asconf->transport = NULL ?
- What about all the similar NULL assignments in sctp_assoc_rm_peer()?
--
Best regards,
Alexander Sverdlin.
next reply other threads:[~2020-12-08 21:01 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-08 20:59 Alexander Sverdlin [this message] 2021-05-12 16:08 ` Race of sctp_assoc_control_transport() against sctp_assoc_rm_peer() ? Xin Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86d2a00b-7fba-a539-b1bf-5bacf0443542@nokia.com \
--to=alexander.sverdlin@nokia.com \
--cc=linux-sctp@vger.kernel.org \
--cc=marcelo.leitner@gmail.com \
--cc=nhorman@tuxdriver.com \
--cc=oss@malat.biz \
--cc=vyasevich@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).