On Fri, Apr 04 2025 at 10:22:38, Xin Long lucien.xin@gmail.com wrote:
Something like this:
@@ -9225,7 +9227,9 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p, pr_debug("%s: asoc:%p, timeo:%ld, msg_len:%zu\n", __func__, asoc, *timeo_p, msg_len);
/* Increment the association's refcnt. */
/* Increment the transport and association's refcnt. */if (transport)sctp_transport_hold(transport); sctp_association_hold(asoc); /* Wait on the association specific sndbuf space. */@@ -9252,6 +9256,8 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p, lock_sock(sk); if (sk != asoc->base.sk) goto do_error;
if (transport && transport->dead)goto do_nonblock; *timeo_p = current_timeo; }@@ -9259,7 +9265,9 @@ static int sctp_wait_for_sndbuf(struct sctp_association *asoc, long *timeo_p, out: finish_wait(&asoc->wait, &wait);
/* Release the association's refcnt. */
/* Release the transport and association's refcnt. */if (transport)sctp_transport_put(transport); sctp_association_put(asoc); return err;So by the time the sending thread re-claims the socket lock it can tell whether someone else removed the transport by checking transport->dead (set in sctp_transport_free()) and there's a guarantee that the transport hasn't been freed yet because we hold a reference to it.
If the whole receive path through sctp_assoc_rm_peer() is protected by the same socket lock, as you said, this should be safe. The tests I ran seem to work fine. If you're ok with it I'll send another patch to supersede this one.
LGTM.
Good, thanks! I submitted a patch that supersedes this one: https://lore.kernel.org/linux-sctp/20250404-kasan_slab-use-after-free_read_i... so we can drop this.
Cheers, Ricardo