[PATCH] nvmet-fc: don't drop the last assoc reference in nvmet_fc_delete_ctrl()

[PATCH] nvmet-fc: don't drop the last assoc reference in nvmet_fc_delete_ctrl()

[Palla Raghunath]

syzbot hit a recursive locking report on subsys->lock when a subsystem
is unlinked from an FC port.

nvmet_port_del_ctrls() holds subsys->lock while it calls ->delete_ctrl()
for each controller. nvmet_fc_delete_ctrl() grabs a reference on the
association, queues del_work and then drops the reference. If del_work
gets to run to completion before that put, ours is the last reference.
Freeing the assoc tears down the queues, the last nvmet_cq_put() drops
the last ctrl reference, and nvmet_ctrl_free() goes for subsys->lock
again:

  WARNING: possible recursive locking detected
  ...
   nvmet_ctrl_free+0xa6/0x8e0 drivers/nvme/target/core.c:1766
   nvmet_cq_put+0x158/0x200 drivers/nvme/target/core.c:848
   nvmet_fc_target_assoc_free+0x418/0x2220 drivers/nvme/target/fc.c:1173
   nvmet_fc_delete_ctrl+0x717/0x7e0 drivers/nvme/target/fc.c:1581
   nvmet_port_del_ctrls+0xfb/0x150 drivers/nvme/target/core.c:311
   nvmet_port_subsys_drop_link+0x1e8/0x310 drivers/nvme/target/configfs.c:1119

It's a real hang, not only a lockdep warning. I reproduced it with
fcloop by putting an msleep() before the put: rm on the configfs link
gets stuck in D state for good.

I don't think we need that reference at all. del_work does
list_del_rcu() and synchronize_rcu() before it drops the initial
reference, so an assoc we find on assoc_list can't go away before
rcu_read_unlock(). nvmet_fc_schedule_delete_assoc() doesn't sleep, so
just call it inside the RCU section and get rid of the get/put. If the
assoc is already being deleted, terminating is set and the call is a
no-op, which matches what happened before when the get failed.

With the same msleep() in place the patched kernel unlinks cleanly and
the assoc is freed from the workqueue.

Fixes: 4049dc96b8de ("nvmet-fc: defer cleanup using RCU properly")
Reported-by: syzbot+da94a51633758d13bd63@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=da94a51633758d13bd63
Signed-off-by: Palla Raghunath <raghunathpalla.0209@gmail.com>
---
 drivers/nvme/target/fc.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1b557775e033..eecdffefa24f 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1567,8 +1567,14 @@ nvmet_fc_delete_ctrl(struct nvmet_ctrl *ctrl)
 		list_for_each_entry_rcu(assoc, &tgtport->assoc_list, a_list) {
 			queue = assoc->queues[0];
 			if (queue && queue->nvme_sq.ctrl == ctrl) {
-				if (nvmet_fc_tgt_a_get(assoc))
-					found_ctrl = true;
+				/*
+				 * No assoc reference here: we're called with
+				 * subsys->lock held, and if del_work beat us,
+				 * our put would free the ctrl under that lock.
+				 * RCU keeps the assoc alive until unlock.
+				 */
+				nvmet_fc_schedule_delete_assoc(assoc);
+				found_ctrl = true;
 				break;
 			}
 		}
@@ -1576,11 +1582,8 @@ nvmet_fc_delete_ctrl(struct nvmet_ctrl *ctrl)
 
 		nvmet_fc_tgtport_put(tgtport);
 
-		if (found_ctrl) {
-			nvmet_fc_schedule_delete_assoc(assoc);
-			nvmet_fc_tgt_a_put(assoc);
+		if (found_ctrl)
 			return;
-		}
 
 		spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
 	}
-- 
2.34.1

Re: [PATCH] nvmet-fc: don't drop the last assoc reference in nvmet_fc_delete_ctrl()

[Daniel Wagner]

On Thu, Oct 08, 2026 at 08:18:48PM +0100, Palla Raghunath wrote:
> syzbot hit a recursive locking report on subsys->lock when a subsystem
> is unlinked from an FC port.
> 
> nvmet_port_del_ctrls() holds subsys->lock while it calls ->delete_ctrl()
> for each controller. nvmet_fc_delete_ctrl() grabs a reference on the
> association, queues del_work and then drops the reference. If del_work
> gets to run to completion before that put, ours is the last reference.
> Freeing the assoc tears down the queues, the last nvmet_cq_put() drops
> the last ctrl reference, and nvmet_ctrl_free() goes for subsys->lock
> again:
> 
>   WARNING: possible recursive locking detected
>   ...
>    nvmet_ctrl_free+0xa6/0x8e0 drivers/nvme/target/core.c:1766
>    nvmet_cq_put+0x158/0x200 drivers/nvme/target/core.c:848
>    nvmet_fc_target_assoc_free+0x418/0x2220 drivers/nvme/target/fc.c:1173
>    nvmet_fc_delete_ctrl+0x717/0x7e0 drivers/nvme/target/fc.c:1581
>    nvmet_port_del_ctrls+0xfb/0x150 drivers/nvme/target/core.c:311
>    nvmet_port_subsys_drop_link+0x1e8/0x310 drivers/nvme/target/configfs.c:1119
> 
> It's a real hang, not only a lockdep warning. I reproduced it with
> fcloop by putting an msleep() before the put: rm on the configfs link
> gets stuck in D state for good.
> 
> I don't think we need that reference at all.

Yes, you are correct. The reference does not protect from del_work
getting scheduled. Indeed this is buggy and your fix looks correct.

Thanks!

Reviewed-by: Daniel Wagner <dwagner@suse.de>

Re: [PATCH] nvmet-fc: don't drop the last assoc reference in nvmet_fc_delete_ctrl()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] nvmet-fc: don't drop the last assoc reference in nvmet_fc_delete_ctrl()

[Keith Busch]

On Thu, Oct 08, 2026 at 08:18:48PM +0100, Palla Raghunath wrote:
> syzbot hit a recursive locking report on subsys->lock when a subsystem
> is unlinked from an FC port.
> 
> nvmet_port_del_ctrls() holds subsys->lock while it calls ->delete_ctrl()
> for each controller. nvmet_fc_delete_ctrl() grabs a reference on the
> association, queues del_work and then drops the reference. If del_work
> gets to run to completion before that put, ours is the last reference.
> Freeing the assoc tears down the queues, the last nvmet_cq_put() drops
> the last ctrl reference, and nvmet_ctrl_free() goes for subsys->lock
> again:

Thanks, applied to nvme-7.4.