Skip to content

Commit f1a69a9

Browse files
r-c-nPaolo Abeni
authored andcommitted
sctp: detect and prevent references to a freed transport in sendmsg
sctp_sendmsg() re-uses associations and transports when possible by doing a lookup based on the socket endpoint and the message destination address, and then sctp_sendmsg_to_asoc() sets the selected transport in all the message chunks to be sent. There's a possible race condition if another thread triggers the removal of that selected transport, for instance, by explicitly unbinding an address with setsockopt(SCTP_SOCKOPT_BINDX_REM), after the chunks have been set up and before the message is sent. This can happen if the send buffer is full, during the period when the sender thread temporarily releases the socket lock in sctp_wait_for_sndbuf(). This causes the access to the transport data in sctp_outq_select_transport(), when the association outqueue is flushed, to result in a use-after-free read. This change avoids this scenario by having sctp_transport_free() signal the freeing of the transport, tagging it as "dead". In order to do this, the patch restores the "dead" bit in struct sctp_transport, which was removed in commit 47faa1e ("sctp: remove the dead field of sctp_transport"). Then, in the scenario where the sender thread has released the socket lock in sctp_wait_for_sndbuf(), the bit is checked again after re-acquiring the socket lock to detect the deletion. This is done while holding a reference to the transport to prevent it from being freed in the process. If the transport was deleted while the socket lock was relinquished, sctp_sendmsg_to_asoc() will return -EAGAIN to let userspace retry the send. The bug was found by a private syzbot instance (see the error report [1] and the C reproducer that triggers it [2]). Link: https://people.igalia.com/rcn/kernel_logs/20250402__KASAN_slab-use-after-free_Read_in_sctp_outq_select_transport.txt [1] Link: https://people.igalia.com/rcn/kernel_logs/20250402__KASAN_slab-use-after-free_Read_in_sctp_outq_select_transport__repro.c [2] Cc: stable@vger.kernel.org Fixes: df132ef ("sctp: clear the transport of some out_chunk_list chunks in sctp_assoc_rm_peer") Suggested-by: Xin Long <lucien.xin@gmail.com> Signed-off-by: Ricardo Cañuelo Navarro <rcn@igalia.com> Acked-by: Xin Long <lucien.xin@gmail.com> Link: https://patch.msgid.link/20250404-kasan_slab-use-after-free_read_in_sctp_outq_select_transport__20250404-v1-1-5ce4a0b78ef2@igalia.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
1 parent cd23e77 commit f1a69a9

File tree

3 files changed

+18
-9
lines changed

3 files changed

+18
-9
lines changed

include/net/sctp/structs.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,7 @@ struct sctp_transport {
775775

776776
/* Reference counting. */
777777
refcount_t refcnt;
778+
__u32 dead:1,
778779
/* RTO-Pending : A flag used to track if one of the DATA
779780
* chunks sent to this address is currently being
780781
* used to compute a RTT. If this flag is 0,
@@ -784,7 +785,7 @@ struct sctp_transport {
784785
* calculation completes (i.e. the DATA chunk
785786
* is SACK'd) clear this flag.
786787
*/
787-
__u32 rto_pending:1,
788+
rto_pending:1,
788789

789790
/*
790791
* hb_sent : a flag that signals that we have a pending

net/sctp/socket.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@
7272
/* Forward declarations for internal helper functions. */
7373
static bool sctp_writeable(const struct sock *sk);
7474
static void sctp_wfree(struct sk_buff *skb);
75-
static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
76-
size_t msg_len);
75+
static int sctp_wait_for_sndbuf(struct sctp_association *asoc,
76+
struct sctp_transport *transport,
77+
long *timeo_p, size_t msg_len);
7778
static int sctp_wait_for_packet(struct sock *sk, int *err, long *timeo_p);
7879
static int sctp_wait_for_connect(struct sctp_association *, long *timeo_p);
7980
static int sctp_wait_for_accept(struct sock *sk, long timeo);
@@ -1828,7 +1829,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
18281829

18291830
if (sctp_wspace(asoc) <= 0 || !sk_wmem_schedule(sk, msg_len)) {
18301831
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
1831-
err = sctp_wait_for_sndbuf(asoc, &timeo, msg_len);
1832+
err = sctp_wait_for_sndbuf(asoc, transport, &timeo, msg_len);
18321833
if (err)
18331834
goto err;
18341835
if (unlikely(sinfo->sinfo_stream >= asoc->stream.outcnt)) {
@@ -9214,8 +9215,9 @@ void sctp_sock_rfree(struct sk_buff *skb)
92149215

92159216

92169217
/* Helper function to wait for space in the sndbuf. */
9217-
static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
9218-
size_t msg_len)
9218+
static int sctp_wait_for_sndbuf(struct sctp_association *asoc,
9219+
struct sctp_transport *transport,
9220+
long *timeo_p, size_t msg_len)
92199221
{
92209222
struct sock *sk = asoc->base.sk;
92219223
long current_timeo = *timeo_p;
@@ -9225,7 +9227,9 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
92259227
pr_debug("%s: asoc:%p, timeo:%ld, msg_len:%zu\n", __func__, asoc,
92269228
*timeo_p, msg_len);
92279229

9228-
/* Increment the association's refcnt. */
9230+
/* Increment the transport and association's refcnt. */
9231+
if (transport)
9232+
sctp_transport_hold(transport);
92299233
sctp_association_hold(asoc);
92309234

92319235
/* Wait on the association specific sndbuf space. */
@@ -9234,7 +9238,7 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
92349238
TASK_INTERRUPTIBLE);
92359239
if (asoc->base.dead)
92369240
goto do_dead;
9237-
if (!*timeo_p)
9241+
if ((!*timeo_p) || (transport && transport->dead))
92389242
goto do_nonblock;
92399243
if (sk->sk_err || asoc->state >= SCTP_STATE_SHUTDOWN_PENDING)
92409244
goto do_error;
@@ -9259,7 +9263,9 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p,
92599263
out:
92609264
finish_wait(&asoc->wait, &wait);
92619265

9262-
/* Release the association's refcnt. */
9266+
/* Release the transport and association's refcnt. */
9267+
if (transport)
9268+
sctp_transport_put(transport);
92639269
sctp_association_put(asoc);
92649270

92659271
return err;

net/sctp/transport.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ struct sctp_transport *sctp_transport_new(struct net *net,
117117
*/
118118
void sctp_transport_free(struct sctp_transport *transport)
119119
{
120+
transport->dead = 1;
121+
120122
/* Try to delete the heartbeat timer. */
121123
if (del_timer(&transport->hb_timer))
122124
sctp_transport_put(transport);

0 commit comments

Comments
 (0)