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, stop processing, leave the addr_waitq entry queued and re-arm addr_wq_timer to retry after SCTP_ADDRESS_TICK_DELAY. The retry is merged with the existing IPv6 DAD retry into a single mod_timer() outside the loops. 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 <18983559317@163.com> --- net/sctp/protocol.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c index 27c26e12f95d..267dc9b7a0e7 100644 --- a/net/sctp/protocol.c +++ b/net/sctp/protocol.c @@ -603,6 +603,7 @@ static void sctp_addr_wq_timeout_handler(struct timer_list *t) struct net *net = timer_container_of(net, t, sctp.addr_wq_timer); struct sctp_sockaddr_entry *addrw, *temp; struct sctp_sock *sp; + bool retry = false; spin_lock_bh(&net->sctp.addr_wq_lock); @@ -624,15 +625,12 @@ static void sctp_addr_wq_timeout_handler(struct timer_list *t) in6 = (struct in6_addr *)&addrw->a.v6.sin6_addr; if (ipv6_chk_addr(net, in6, NULL, 0) == 0 && addrw->state == SCTP_ADDR_NEW) { - unsigned long timeo_val; pr_debug("%s: this is on DAD, trying %d sec " "later\n", __func__, SCTP_ADDRESS_TICK_DELAY); - timeo_val = jiffies; - timeo_val += msecs_to_jiffies(SCTP_ADDRESS_TICK_DELAY); - mod_timer(&net->sctp.addr_wq_timer, timeo_val); + retry = true; break; } } @@ -645,16 +643,27 @@ 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); + retry = true; + break; + } if (sctp_asconf_mgmt(sp, addrw) < 0) pr_debug("%s: sctp_asconf_mgmt failed\n", __func__); bh_unlock_sock(sk); } + if (retry) + break; #if IS_ENABLED(CONFIG_IPV6) free_next: #endif list_del(&addrw->list); kfree(addrw); } + + if (retry) + mod_timer(&net->sctp.addr_wq_timer, + jiffies + msecs_to_jiffies(SCTP_ADDRESS_TICK_DELAY)); spin_unlock_bh(&net->sctp.addr_wq_lock); } -- 2.43.0