[PATCH] net/rds: ib: don't enable IRQs in rds_ib_conn_free()

[PATCH] net/rds: ib: don't enable IRQs in rds_ib_conn_free()

[Nguyen Ngoc Thang]

__rds_conn_create() calls trans->conn_free() with rds_conn_lock held
and IRQs disabled when it loses the race to insert a connection.
rds_ib_conn_free() takes the device list lock with spin_lock_irq() and
releases it with spin_unlock_irq(), so IRQs are turned on while
rds_conn_lock is still held:

  raw_local_irq_restore() called with IRQs enabled
  WARNING: kernel/locking/irqflag-debug.c:10 at warn_bogus_irq_restore+0xc/0x20
   _raw_spin_unlock_irqrestore+0x67/0x80
   __rds_conn_create+0x1de5/0x2290
   rds_conn_create_outgoing+0x43/0x60
   rds_sendmsg+0xfca/0x20e0
   __sys_sendto+0x408/0x5a0

It needs an RDS socket bound to an address on an RDMA device (e.g. rxe)
and two concurrent sendto() calls to the same new destination.

The TCP and loopback ->conn_free() callbacks already use irqsave, so
do the same here.

Fixes: 745cbccac3fe ("RDS: Rewrite connection cleanup, fixing oops on rmmod")
Reported-by: syzbot+c3741bb338b5c6d6a4aa@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c3741bb338b5c6d6a4aa
Cc: stable@vger.kernel.org
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
 net/rds/ib_cm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 6e3110a04ae6..5c717120eab5 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -1271,6 +1271,7 @@ void rds_ib_conn_free(void *arg)
 {
 	struct rds_ib_connection *ic = arg;
 	spinlock_t	*lock_ptr;
+	unsigned long flags;
 
 	rdsdebug("ic %p\n", ic);
 
@@ -1281,9 +1282,10 @@ void rds_ib_conn_free(void *arg)
 	 */
 	lock_ptr = ic->rds_ibdev ? &ic->rds_ibdev->spinlock : &ib_nodev_conns_lock;
 
-	spin_lock_irq(lock_ptr);
+	/* May be called with IRQs already disabled (see __rds_conn_create()) */
+	spin_lock_irqsave(lock_ptr, flags);
 	list_del(&ic->ib_node);
-	spin_unlock_irq(lock_ptr);
+	spin_unlock_irqrestore(lock_ptr, flags);
 
 	rds_ib_recv_free_caches(ic);
 
-- 
2.43.0

Re: [PATCH] net/rds: ib: don't enable IRQs in rds_ib_conn_free()

[sashiko-bot]

> __rds_conn_create() calls trans->conn_free() with rds_conn_lock held
> and IRQs disabled when it loses the race to insert a connection.
> rds_ib_conn_free() takes the device list lock with spin_lock_irq() and
> releases it with spin_unlock_irq(), so IRQs are turned on while
> rds_conn_lock is still held:
> 
>   raw_local_irq_restore() called with IRQs enabled
>   WARNING: kernel/locking/irqflag-debug.c:10 at warn_bogus_irq_restore+0xc/0x20
>    _raw_spin_unlock_irqrestore+0x67/0x80
>    __rds_conn_create+0x1de5/0x2290
>    rds_conn_create_outgoing+0x43/0x60
>    rds_sendmsg+0xfca/0x20e0
>    __sys_sendto+0x408/0x5a0
> 
> It needs an RDS socket bound to an address on an RDMA device (e.g. rxe)
> and two concurrent sendto() calls to the same new destination.
> 
> The TCP and loopback ->conn_free() callbacks already use irqsave, so
> do the same here.
> 
> Fixes: 745cbccac3fe ("RDS: Rewrite connection cleanup, fixing oops on rmmod")
> Reported-by: syzbot+c3741bb338b5c6d6a4aa@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c3741bb338b5c6d6a4aa
> Cc: stable@vger.kernel.org
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016161525.146537-1-ngocthang2710.1999@gmail.com?part=1

Re: [PATCH] net/rds: ib: don't enable IRQs in rds_ib_conn_free()

[Allison Henderson]

On Fri, 2026-10-16 at 23:15 +0700, Nguyen Ngoc Thang wrote:
> __rds_conn_create() calls trans->conn_free() with rds_conn_lock held
> and IRQs disabled when it loses the race to insert a connection.
> rds_ib_conn_free() takes the device list lock with spin_lock_irq() and
> releases it with spin_unlock_irq(), so IRQs are turned on while
> rds_conn_lock is still held:
> 
>   raw_local_irq_restore() called with IRQs enabled
>   WARNING: kernel/locking/irqflag-debug.c:10 at warn_bogus_irq_restore+0xc/0x20
>    _raw_spin_unlock_irqrestore+0x67/0x80
>    __rds_conn_create+0x1de5/0x2290
>    rds_conn_create_outgoing+0x43/0x60
>    rds_sendmsg+0xfca/0x20e0
>    __sys_sendto+0x408/0x5a0
> 
> It needs an RDS socket bound to an address on an RDMA device (e.g. rxe)
> and two concurrent sendto() calls to the same new destination.
> 
> The TCP and loopback ->conn_free() callbacks already use irqsave, so
> do the same here.
> 
> Fixes: 745cbccac3fe ("RDS: Rewrite connection cleanup, fixing oops on rmmod")
> Reported-by: syzbot+c3741bb338b5c6d6a4aa@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c3741bb338b5c6d6a4aa
> Cc: stable@vger.kernel.org
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

Hi Nguyen,

Thanks for the fix.  This is the same change as patch 1 of the
connection-lifetime series pending for net-next ("net/rds: ib: don't
enable interrupts in rds_ib_conn_free()").  But that's a much bigger set
that's been churning for a while.  Since yours is stand alone with the
syzbot report for net, it's fine for yours to go first.  I'll drop my
patch 1 when your patch lands.

Reviewed-by: Allison Henderson <achender@kernel.org>

Thanks!
Allison

> ---
>  net/rds/ib_cm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
> index 6e3110a04ae6..5c717120eab5 100644
> --- a/net/rds/ib_cm.c
> +++ b/net/rds/ib_cm.c
> @@ -1271,6 +1271,7 @@ void rds_ib_conn_free(void *arg)
>  {
>  	struct rds_ib_connection *ic = arg;
>  	spinlock_t	*lock_ptr;
> +	unsigned long flags;
>  
>  	rdsdebug("ic %p\n", ic);
>  
> @@ -1281,9 +1282,10 @@ void rds_ib_conn_free(void *arg)
>  	 */
>  	lock_ptr = ic->rds_ibdev ? &ic->rds_ibdev->spinlock : &ib_nodev_conns_lock;
>  
> -	spin_lock_irq(lock_ptr);
> +	/* May be called with IRQs already disabled (see __rds_conn_create()) */
> +	spin_lock_irqsave(lock_ptr, flags);
>  	list_del(&ic->ib_node);
> -	spin_unlock_irq(lock_ptr);
> +	spin_unlock_irqrestore(lock_ptr, flags);
>  
>  	rds_ib_recv_free_caches(ic);
>