[PATCH net] rxrpc: Prevent poking connections after final put

Chengfeng Ye posted 1 patch 2 months ago
net/rxrpc/conn_object.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH net] rxrpc: Prevent poking connections after final put
Posted by Chengfeng Ye 2 months ago
rxrpc_poke_conn() unconditionally gets a reference before adding a
connection to conn_attend_q.  This is unsafe for the connection timer
because timer deletion cannot stop a callback that has already started.

The following interleaving can occur:

  I/O thread                         Timer softirq
  rxrpc_put_connection()
    refcount reaches zero
                                     rxrpc_connection_timer()
                                       rxrpc_poke_conn()
                                         rxrpc_get_connection()
                                           increment from zero
                                         list_add_tail()
    timer_delete()
    rxrpc_clean_up_connection()
      timer_delete_sync()
      call_rcu()

The refcount increment saturates instead of resurrecting the connection,
so cleanup continues after the timer callback exits.  The RCU callback can
then free the connection while attend_link remains queued, and the I/O
thread later accesses the stale queue entry.

The kernel reported:

  refcount_t: addition on 0; use-after-free.
  WARNING: lib/refcount.c:25 at refcount_warn_saturate+0xc0/0xe0
  Call Trace:
   <IRQ>
   rxrpc_get_connection+0x185/0x1d0
   rxrpc_poke_conn+0x12e/0x2b0
   call_timer_fn+0x2b/0x220
   __run_timers+0x561/0x8d0
   run_timer_softirq+0x144/0x250
   handle_softirqs+0x18d/0x5b0
   </IRQ>
   <TASK>
   rxrpc_put_connection+0x2fe/0x3f0
   rxrpc_discard_expired_client_conns+0x396/0x670
   rxrpc_io_thread+0xbfc/0x1640
   </TASK>

Use rxrpc_get_connection_maybe() while holding the attend queue lock and
only enqueue the connection if a live reference was acquired.  The normal
path for a live connection is unchanged.

Fixes: 4241a702e0d0 ("rxrpc: Fix the rxrpc_connection attend queue handling")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/rxrpc/conn_object.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index 0ece717db0f8..3f4516115d37 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -33,10 +33,8 @@ void rxrpc_poke_conn(struct rxrpc_connection *conn, enum rxrpc_conn_trace why)
 
 	spin_lock_irq(&local->lock);
 	busy = !list_empty(&conn->attend_link);
-	if (!busy) {
-		rxrpc_get_connection(conn, why);
+	if (!busy && rxrpc_get_connection_maybe(conn, why))
 		list_add_tail(&conn->attend_link, &local->conn_attend_q);
-	}
 	spin_unlock_irq(&local->lock);
 	rxrpc_wake_up_io_thread(local);
 }
-- 
2.43.0
Re: [PATCH net] rxrpc: Prevent poking connections after final put
Posted by Chengfeng Ye 4 days, 23 hours ago
On Fri, Jul 31, 2026 at 11:14 PM Chengfeng Ye <nicoyip.dev@gmail.com> wrote:
>
> rxrpc_poke_conn() unconditionally gets a reference before adding a
> connection to conn_attend_q.  This is unsafe for the connection timer
> because timer deletion cannot stop a callback that has already started.
>
> The following interleaving can occur:
>
>   I/O thread                         Timer softirq
>   rxrpc_put_connection()
>     refcount reaches zero
>                                      rxrpc_connection_timer()
>                                        rxrpc_poke_conn()
>                                          rxrpc_get_connection()
>                                            increment from zero
>                                          list_add_tail()
>     timer_delete()
>     rxrpc_clean_up_connection()
>       timer_delete_sync()
>       call_rcu()
>
> The refcount increment saturates instead of resurrecting the connection,
> so cleanup continues after the timer callback exits.  The RCU callback can
> then free the connection while attend_link remains queued, and the I/O
> thread later accesses the stale queue entry.
>
> The kernel reported:
>
>   refcount_t: addition on 0; use-after-free.
>   WARNING: lib/refcount.c:25 at refcount_warn_saturate+0xc0/0xe0
>   Call Trace:
>    <IRQ>
>    rxrpc_get_connection+0x185/0x1d0
>    rxrpc_poke_conn+0x12e/0x2b0
>    call_timer_fn+0x2b/0x220
>    __run_timers+0x561/0x8d0
>    run_timer_softirq+0x144/0x250
>    handle_softirqs+0x18d/0x5b0
>    </IRQ>
>    <TASK>
>    rxrpc_put_connection+0x2fe/0x3f0
>    rxrpc_discard_expired_client_conns+0x396/0x670
>    rxrpc_io_thread+0xbfc/0x1640
>    </TASK>
>
> Use rxrpc_get_connection_maybe() while holding the attend queue lock and
> only enqueue the connection if a live reference was acquired.  The normal
> path for a live connection is unchanged.
>
> Fixes: 4241a702e0d0 ("rxrpc: Fix the rxrpc_connection attend queue handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
>  net/rxrpc/conn_object.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
> index 0ece717db0f8..3f4516115d37 100644
> --- a/net/rxrpc/conn_object.c
> +++ b/net/rxrpc/conn_object.c
> @@ -33,10 +33,8 @@ void rxrpc_poke_conn(struct rxrpc_connection *conn, enum rxrpc_conn_trace why)
>
>         spin_lock_irq(&local->lock);
>         busy = !list_empty(&conn->attend_link);
> -       if (!busy) {
> -               rxrpc_get_connection(conn, why);
> +       if (!busy && rxrpc_get_connection_maybe(conn, why))
>                 list_add_tail(&conn->attend_link, &local->conn_attend_q);
> -       }
>         spin_unlock_irq(&local->lock);
>         rxrpc_wake_up_io_thread(local);
>  }
> --
> 2.43.0
>

Hi David,

I noticed that an equivalent patch from Seungwon Bae, “rxrpc: fix
use-after-free in rxrpc_poke_conn()”, was included in your fixes
series:

https://lore.kernel.org/r/20260923133706.1496540-18-dhowells@redhat.com/

It appears to address the same race using
rxrpc_get_connection_maybe(). Since I reported this race and submitted
an equivalent fix on July 31, I would appreciate it if there could be
a Reported-by tag, or Co-developed-by tag for me.

Thanks.
Chengfeng