[PATCH] nvmet-fc: extend critical section in nvmet_fc_remove_port()

[PATCH] nvmet-fc: extend critical section in nvmet_fc_remove_port()

[paritoshpotukuchi]

From: Paritosh Potukuchi <paritosh.potukuchi@amd.com>

Currently in nvmet_fc_remove_port the critical section starts from
the call to spin_lock_irqsave(&nvmet_fc_tgtlock, flags) and ends
before the call to nvmet_fc_portentry_unbind(pe).

nvmet_fc_portentry_unbind takes a spin_lock_irqsave on nvmet_fc_tgtlock
again, to execute its critical section. The problem here is that,
releasing the lock before calling nvmet_fc_portentry_unbind, and then
retaking the lock inside it, creates room for other threads to possibly
enter the critical section.

The solution proposed is to extend the critical section inside
nvmet_fc_remove_port, till the point where tgtport is being used.

Reported-by: syzbot+df0058a8d0bdce7c4059@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=df0058a8d0bdce7c4059
Co-developed-by: Ravi Kumar <ravi.kumar3@amd.com>
Signed-off-by: Ravi Kumar <ravi.kumar3@amd.com>
Signed-off-by: Paritosh Potukuchi <paritosh.potukuchi@amd.com>
---
 drivers/nvme/target/fc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1b557775e033..96d8aafb9f56 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1271,15 +1271,11 @@ nvmet_fc_portentry_bind(struct nvmet_fc_tgtport *tgtport,
 static void
 nvmet_fc_portentry_unbind(struct nvmet_fc_port_entry *pe)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
 	if (pe->tgtport) {
 		nvmet_fc_tgtport_put(pe->tgtport);
 		pe->tgtport->pe = NULL;
 	}
 	list_del(&pe->pe_list);
-	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
 }
 
 /*
@@ -2922,7 +2918,6 @@ nvmet_fc_remove_port(struct nvmet_port *port)
 	spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
 	if (pe->tgtport && nvmet_fc_tgtport_get(pe->tgtport))
 		tgtport = pe->tgtport;
-	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
 
 	nvmet_fc_portentry_unbind(pe);
 
@@ -2932,6 +2927,8 @@ nvmet_fc_remove_port(struct nvmet_port *port)
 		nvmet_fc_tgtport_put(tgtport);
 	}
 
+	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
+
 	kfree(pe);
 }
 
-- 
2.55.0

Re: [PATCH] nvmet-fc: extend critical section in nvmet_fc_remove_port()

[Maurizio Lombardi]

On Tue Oct 27, 2026 at 3:43 PM CET, paritoshpotukuchi wrote:
> @@ -2932,6 +2927,8 @@ nvmet_fc_remove_port(struct nvmet_port *port)
>  		nvmet_fc_tgtport_put(tgtport);
>  	}
>  
> +	spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
> +
>  	kfree(pe);
>  }
>

I don't think this safe.
nvmet_fc_tgtport_put() can end up calling fcloop_targetport_delete()
which calls flush_work(), therefore you call a blocking function while
in atomic context

Maurizio

Re: [PATCH] nvmet-fc: extend critical section in nvmet_fc_remove_port()

[paritoshpotukuchi]

> I don't think this safe.
> nvmet_fc_tgtport_put() can end up calling fcloop_targetport_delete()
> which calls flush_work(), therefore you call a blocking function while
> in atomic context

Thanks for pointing that out. 

The current implementation of nvmet_fc_portentry_unbind calls 
nvmet_fc_tgtport_put() while holding a spinlock.

From what we understand, this seems to be a design flaw,
since flush_work() is still being called while holding a spinlock.


Paritosh