[PATCH] net/rds: hold IB device reference through deferred cleanup

[PATCH] net/rds: hold IB device reference through deferred cleanup

[Eduard Zateev]

RDS device teardown queues rds_ib_dev_free() on rds_wq. Hold a
reference to the underlying IB device until deferred cleanup releases
its resources. This prevents device removal from freeing it while the
work is running.

Reported-by: syzbot+221e213bf17f17e0d6cd@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=221e213bf17f17e0d6cd
Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
---
 net/rds/ib.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/rds/ib.c b/net/rds/ib.c
index 786f39169bc..597872e4fe9 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -115,6 +115,7 @@ static void rds_ib_dev_free(struct work_struct *work)
 
 	kfree(rds_ibdev->vector_load);
 
+	ib_device_put(rds_ibdev->dev);
 	kfree(rds_ibdev);
 }
 
@@ -142,6 +143,12 @@ static int rds_ib_add_one(struct ib_device *device)
 				 ibdev_to_node(device));
 	if (!rds_ibdev)
 		return -ENOMEM;
+	/* Keep the device registered until the deferred cleanup completes. */
+	if (!ib_device_try_get(device)) {
+		kfree(rds_ibdev);
+		return -ENODEV;
+	}
+	rds_ibdev->dev = device;
 
 	spin_lock_init(&rds_ibdev->spinlock);
 	refcount_set(&rds_ibdev->refcount, 1);
@@ -180,7 +187,6 @@ static int rds_ib_add_one(struct ib_device *device)
 		goto put_dev;
 	}
 
-	rds_ibdev->dev = device;
 	rds_ibdev->pd = ib_alloc_pd(device, 0);
 	if (IS_ERR(rds_ibdev->pd)) {
 		ret = PTR_ERR(rds_ibdev->pd);
-- 
2.55.0

Re: [PATCH] net/rds: hold IB device reference through deferred cleanup

[Allison Henderson]

On Fri, 2026-10-23 at 12:02 +0200, Eduard Zateev wrote:
> RDS device teardown queues rds_ib_dev_free() on rds_wq. Hold a
> reference to the underlying IB device until deferred cleanup releases
> its resources. This prevents device removal from freeing it while the
> work is running.

Hi Eduard,

Thanks for the patch.  Some comments below:

> 
> Reported-by: syzbot+221e213bf17f17e0d6cd@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=221e213bf17f17e0d6cd
The report referenced here doesnt actually point to RDS, it's a warning in rxe_pool_cleanup.
Do you have a trace or a syz test result showing that the leftover rxe object is the RDS PD
or MR and that this patch resolves the warning? Otherwise, I think the closes tag should be
removed.

> Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
> ---
>  net/rds/ib.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/rds/ib.c b/net/rds/ib.c
> index 786f39169bc..597872e4fe9 100644
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -115,6 +115,7 @@ static void rds_ib_dev_free(struct work_struct *work)
>  
>  	kfree(rds_ibdev->vector_load);
>  
> +	ib_device_put(rds_ibdev->dev);
>  	kfree(rds_ibdev);
>  }
>  
> @@ -142,6 +143,12 @@ static int rds_ib_add_one(struct ib_device *device)
>  				 ibdev_to_node(device));
>  	if (!rds_ibdev)
>  		return -ENOMEM;
> +	/* Keep the device registered until the deferred cleanup completes. */
> +	if (!ib_device_try_get(device)) {


The commit message doesn't quite describe what this does. Holding the reference doesn't stop
the device from being freed while the work runs. It makes ib_unregister_device() block in
disable_device() until the refcount reaches zero, which now only happens when rds_ib_dev_free()
calls ib_device_put(). For connections that wait is short, since rdma_cm tears them down during
the same removal, but connections are not the only things that hold the rds_ib_device.

A registered MR keeps the reference it takes in rds_ib_get_mr() until rds_ib_free_mr() drops
it. That only happens once everything using the MR has let go. Including the socket (when it
frees the MR or closes), and every queued message that uses it (when the peer acks it or the
socket closes).  Those messages also survive connection drops (rds_send_path_reset() requeues
them), and with the  device gone the connection can't come back up over it to get them acked. 

So with this patch, whoever is unregistering the device (rdma link delete, driver unload, PCI
hot-unplug) blocks in disable_device() until the process that owns the socket lets go, which
it may never do. It  also turns any leaked rds_ib_device reference into a permanent unregister
hang. I have a fix for one such leak, a connection left attached at module unload, in the
connection-lifetime series I've been iterating on, but it hasn't landed yet.

Unfortunately this is not a simple fix, even after kref lands. To wait for the free safely,
remove_one first needs a way to make the device's MRs be released, so that the wait is
guaranteed to finish.  The uek tree approach is a socket accounting feature:

https://github.com/oracle/linux-uek/commit/ba92066dbf13e573a292252ea07e686b5104f091
https://github.com/oracle/linux-uek/commit/dc4c8c665368bca276db02ba7071d2d3fd653a10
https://github.com/oracle/linux-uek/commit/fcd394a1095625a39c44aa709bd976c6b9b98373

You're certainly welcome to look at it, but it is a bigger rework of the removal path than
this patch, and I'd rather not take a partial fix that turns the warning into a hang.

Thank you for looking at this though!

Allison

> +		kfree(rds_ibdev);
> +		return -ENODEV;
> +	}
> +	rds_ibdev->dev = device;
>  
>  	spin_lock_init(&rds_ibdev->spinlock);
>  	refcount_set(&rds_ibdev->refcount, 1);
> @@ -180,7 +187,6 @@ static int rds_ib_add_one(struct ib_device *device)
>  		goto put_dev;
>  	}
>  
> -	rds_ibdev->dev = device;
>  	rds_ibdev->pd = ib_alloc_pd(device, 0);
>  	if (IS_ERR(rds_ibdev->pd)) {
>  		ret = PTR_ERR(rds_ibdev->pd);