[PATCH net-next] sctp: fix use-after-free in timeout event handling

luoqing posted 1 patch 1 month, 4 weeks ago
net/sctp/sm_sideeffect.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
[PATCH net-next] sctp: fix use-after-free in timeout event handling
Posted by luoqing 1 month, 4 weeks ago
From: Qing Luo <luoqing@kylinos.cn>

sctp_do_sm() in sctp_generate_timeout_event() may process an
SCTP_CMD_ASSOC_FAILED command, which calls sctp_association_free() and
drops the last reference, freeing the association and its socket while
the timer callback is still running. The callback then dereferences the
freed sk and asoc, a use-after-free.

Hold an extra reference on the association at the start of the callback
so both it and its socket stay alive until we are done, and release it
at out_unlock along with the timer's reference. Also only set sk->sk_err
while the association is still alive.

Assisted-by: LLM
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 net/sctp/sm_sideeffect.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 424f10a6fdba..f384c2faf1c4 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -275,6 +275,12 @@ static void sctp_generate_timeout_event(struct sctp_association *asoc,
 	int error = 0;
 
 	bh_lock_sock(sk);
+
+	/* Take an extra ref to keep asoc alive through sctp_do_sm().
+	 * The timer already holds one ref; we'll release both at out_unlock.
+	 */
+	sctp_association_hold(asoc);
+
 	if (sock_owned_by_user(sk)) {
 		pr_debug("%s: sock is busy: timer %d\n", __func__,
 			 timeout_type);
@@ -296,13 +302,13 @@ static void sctp_generate_timeout_event(struct sctp_association *asoc,
 			   SCTP_ST_TIMEOUT(timeout_type),
 			   asoc->state, asoc->ep, asoc,
 			   (void *)timeout_type, GFP_ATOMIC);
-
-	if (error)
+	if (!asoc->base.dead && error)
 		sk->sk_err = -error;
 
 out_unlock:
 	bh_unlock_sock(sk);
-	sctp_association_put(asoc);
+	sctp_association_put(asoc); 	/* release timer's ref */
+	sctp_association_put(asoc);	/* release our extra ref */
 }
 
 static void sctp_generate_t1_cookie_event(struct timer_list *t)
-- 
2.25.1
Re: [PATCH net-next] sctp: fix use-after-free in timeout event handling
Posted by Xin Long 1 month, 4 weeks ago
On Tue, Aug 4, 2026 at 4:44 AM luoqing <l1138897701@163.com> wrote:
>
> From: Qing Luo <luoqing@kylinos.cn>
>
> sctp_do_sm() in sctp_generate_timeout_event() may process an
> SCTP_CMD_ASSOC_FAILED command, which calls sctp_association_free() and
> drops the last reference, freeing the association and its socket while
> the timer callback is still running. The callback then dereferences the
> freed sk and asoc, a use-after-free.
>
I don't understand how this happened.

If the timer callback is still running, the asoc refcnt must be held until the
callback completes, and sctp_association_destroy() is not supposed to
put the refcnt any running timer holds.

Can you share the PoC and the call trace if any?

Thanks.

> Hold an extra reference on the association at the start of the callback
> so both it and its socket stay alive until we are done, and release it
> at out_unlock along with the timer's reference. Also only set sk->sk_err
> while the association is still alive.
>
> Assisted-by: LLM
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
>  net/sctp/sm_sideeffect.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 424f10a6fdba..f384c2faf1c4 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -275,6 +275,12 @@ static void sctp_generate_timeout_event(struct sctp_association *asoc,
>         int error = 0;
>
>         bh_lock_sock(sk);
> +
> +       /* Take an extra ref to keep asoc alive through sctp_do_sm().
> +        * The timer already holds one ref; we'll release both at out_unlock.
> +        */
> +       sctp_association_hold(asoc);
> +
>         if (sock_owned_by_user(sk)) {
>                 pr_debug("%s: sock is busy: timer %d\n", __func__,
>                          timeout_type);
> @@ -296,13 +302,13 @@ static void sctp_generate_timeout_event(struct sctp_association *asoc,
>                            SCTP_ST_TIMEOUT(timeout_type),
>                            asoc->state, asoc->ep, asoc,
>                            (void *)timeout_type, GFP_ATOMIC);
> -
> -       if (error)
> +       if (!asoc->base.dead && error)
>                 sk->sk_err = -error;
>
>  out_unlock:
>         bh_unlock_sock(sk);
> -       sctp_association_put(asoc);
> +       sctp_association_put(asoc);     /* release timer's ref */
> +       sctp_association_put(asoc);     /* release our extra ref */
>  }
>
>  static void sctp_generate_t1_cookie_event(struct timer_list *t)
> --
> 2.25.1
>