sctp_addr_wq_timeout_handler() walks net->sctp.auto_asconf_splist and,
holding only bh_lock_sock(), calls sctp_asconf_mgmt() on every
auto-asconf socket. Unlike every other asynchronous SCTP entry point
(sctp_rcv() queuing packets to the backlog, and the timer handlers in
sctp_sm_side_effect()), it does not check sock_owned_by_user() to defer
its work while a user context owns the socket.
While a user thread owns the socket (e.g. recvmsg() blocked in a copy
to user, or release_sock() draining the backlog), sk_lock.owned is set
but the socket spinlock has been dropped, so bh_lock_sock() succeeds
and the timer enters
sctp_asconf_mgmt() -> sctp_send_asconf_add/del_ip() -> sctp_do_sm()
concurrently with the backlog drain path
release_sock() -> __release_sock() -> sk_backlog_rcv()
-> sctp_assoc_bh_rcv() -> sctp_do_sm()
Two state machines then run on the same association with no common
serialization and mutate shared state concurrently: the outqueue
control chunk list, transport->packet.chunk_list and packet->auth, and
the transport/dst lifetime.
The bug was found by code review of this timer entry point against the
sock_owned_by_user() avoidance used by all other SCTP asynchronous
entries, and confirmed with a reproducer that races an RTM_DELADDR
address event against a user-owned auto-asconf socket. It triggers the
following, observed with KASAN and refcount debugging:
- rcuref "imbalanced put()" from dst_release() in
sctp_transport_route() racing the timer's transport routing,
- a GPF in sctp_packet_transmit() dereferencing a chunk pointer
corrupted by concurrent packet/chunk list use, and
- associations dying silently (ETIMEDOUT, GET_PEER_ADDRS failure).
Defer the work like the other entries do: if the socket is owned by
the user, drop both locks, leave the addr_waitq entry queued and
re-arm addr_wq_timer to retry after SCTP_ADDRESS_TICK_DELAY. Sockets
processed earlier for the same address event will see a duplicate
ASCONF on retry; the peer reports that in an ASCONF-ACK error
indication, which the association tolerates.
Fixes: 9f7d653b67ae ("sctp: Add Auto-ASCONF support (core).")
Signed-off-by: Binbin Deng <redacted>
---
net/sctp/protocol.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 27c26e12f95d..7528a40a87f5 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -645,6 +645,14 @@ static void sctp_addr_wq_timeout_handler(struct timer_list *t)
if (!sctp_is_ep_boundall(sk))
continue;
bh_lock_sock(sk);
+ if (sock_owned_by_user(sk)) {
+ bh_unlock_sock(sk);
+ spin_unlock_bh(&net->sctp.addr_wq_lock);
+ /* Try again later. */
+ mod_timer(&net->sctp.addr_wq_timer,
+ jiffies + msecs_to_jiffies(SCTP_ADDRESS_TICK_DELAY));
+ return;
+ }
if (sctp_asconf_mgmt(sp, addrw) < 0)
pr_debug("%s: sctp_asconf_mgmt failed\n", __func__);
bh_unlock_sock(sk);--
2.43.0