[PATCH] nvmet-fcloop: fail LS request synchronously when remote port is gone

Nguyen Ngoc Thang posted 1 patch 4 days, 4 hours ago
drivers/nvme/target/fcloop.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
[PATCH] nvmet-fcloop: fail LS request synchronously when remote port is gone
Posted by Nguyen Ngoc Thang 4 days, 4 hours ago
fcloop_t2h_ls_req() handles a missing remoteport by queueing the request
on tport->ls_list and completing it with -ECONNREFUSED from a work item.

nvmet_fc_delete_assoc_work() sends the Disconnect Association LS this
way. When it runs during nvmet_fc_unregister_targetport(), the work is
queued after flush_workqueue() has started, so it is not waited for.
nvmet_fc_free_pending_reqs() then frees the still-pending lsop and the
late work item calls lsreq->done() on freed memory:

  BUG: KASAN: slab-use-after-free in fcloop_rport_lsrqst_work+0x242/0x2e0
  Workqueue: nvmet-wq fcloop_tport_lsrqst_work
  Allocated by nvmet_fc_xmt_disconnect_assoc
  Freed by nvmet_fc_free_pending_reqs
           nvmet_fc_unregister_targetport
           fcloop_delete_target_port

Return -ECONNREFUSED directly instead. __nvmet_fc_send_ls_req() unwinds
and nvmet_fc_xmt_disconnect_assoc() frees the lsop, so no completion is
left outstanding.

Reported-by: syzbot+77955102efac681ec73b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=77955102efac681ec73b
Fixes: bbccbf791e6f ("nvmet-fc: free pending reqs on tgtport unregister")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
 drivers/nvme/target/fcloop.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index b63af3b643a6..9f9fb40f8a97 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -456,21 +456,18 @@ fcloop_t2h_ls_req(struct nvmet_fc_target_port *targetport, void *hosthandle,
 	 * 1:1 tgtport vs remoteport
 	 */
 
+	/*
+	 * Fail synchronously: an async completion can run after
+	 * nvmet_fc_unregister_targetport() has freed the pending lsreq.
+	 */
+	if (!tport->remoteport)
+		return -ECONNREFUSED;
+
 	tls_req = kmem_cache_alloc(lsreq_cache, GFP_KERNEL);
 	if (!tls_req)
 		return -ENOMEM;
 	tls_req->lsreq = lsreq;
 	INIT_LIST_HEAD(&tls_req->ls_list);
-
-	if (!tport->remoteport) {
-		tls_req->status = -ECONNREFUSED;
-		spin_lock(&tport->lock);
-		list_add_tail(&tls_req->ls_list, &tport->ls_list);
-		spin_unlock(&tport->lock);
-		queue_work(nvmet_wq, &tport->ls_work);
-		return ret;
-	}
-
 	tls_req->status = 0;
 	ret = nvme_fc_rcv_ls_req(tport->remoteport, &tls_req->ls_rsp,
 				 lsreq->rqstaddr, lsreq->rqstlen);
-- 
2.43.0
Re: [PATCH] nvmet-fcloop: fail LS request synchronously when remote port is gone
Posted by Daniel Wagner 3 days, 9 hours ago
On Sun, Sep 20, 2026 at 11:38:04PM +0700, Nguyen Ngoc Thang wrote:
> fcloop_t2h_ls_req() handles a missing remoteport by queueing the request
> on tport->ls_list and completing it with -ECONNREFUSED from a work item.
> 
> nvmet_fc_delete_assoc_work() sends the Disconnect Association LS this
> way. When it runs during nvmet_fc_unregister_targetport(), the work is
> queued after flush_workqueue() has started, so it is not waited for.
> nvmet_fc_free_pending_reqs() then frees the still-pending lsop and the
> late work item calls lsreq->done() on freed memory:
> 
>   BUG: KASAN: slab-use-after-free in fcloop_rport_lsrqst_work+0x242/0x2e0
>   Workqueue: nvmet-wq fcloop_tport_lsrqst_work
>   Allocated by nvmet_fc_xmt_disconnect_assoc
>   Freed by nvmet_fc_free_pending_reqs
>            nvmet_fc_unregister_targetport
>            fcloop_delete_target_port
> 
> Return -ECONNREFUSED directly instead. __nvmet_fc_send_ls_req() unwinds
> and nvmet_fc_xmt_disconnect_assoc() frees the lsop, so no completion is
> left outstanding.

IIRC this has been added to free memory in a different execution
scenario. So I don't think you can just skip it.

And this is also a change in the behavior between HBA's behaviour in the
loopback behavior.
Re: [PATCH] nvmet-fcloop: fail LS request synchronously when remote port is gone
Posted by Nguyen Ngoc Thang 3 days, 5 hours ago
Hi Daniel,

Thanks for the review, you are right: the async -ECONNREFUSED completion
is how an HBA would behave, so failing synchronously in fcloop changes
that.

I dropped the fcloop change. v2 keeps fcloop as is and flushes nvmet_wq a
second time in nvmet_fc_unregister_targetport(), so the LS completion
queued by nvmet_fc_delete_assoc_work() during the first flush has run
before nvmet_fc_free_pending_reqs() frees the remaining requests:

  [PATCH v2] nvmet-fc: flush nvmet_wq twice on targetport unregister
  Message-ID: <20260921145651.17131-1-ngocthang2710.1999@gmail.com>

Hannes' blktest request is covered by nvme/071, sent separately to
linux-block. It fails with a UAF on an unpatched kernel and passes with v2.

Thanks,
Thang
Re: [PATCH] nvmet-fcloop: fail LS request synchronously when remote port is gone
Posted by Hannes Reinecke 3 days, 11 hours ago
On 9/20/26 6:38 PM, Nguyen Ngoc Thang wrote:
> fcloop_t2h_ls_req() handles a missing remoteport by queueing the request
> on tport->ls_list and completing it with -ECONNREFUSED from a work item.
> 
> nvmet_fc_delete_assoc_work() sends the Disconnect Association LS this
> way. When it runs during nvmet_fc_unregister_targetport(), the work is
> queued after flush_workqueue() has started, so it is not waited for.
> nvmet_fc_free_pending_reqs() then frees the still-pending lsop and the
> late work item calls lsreq->done() on freed memory:
> 
>    BUG: KASAN: slab-use-after-free in fcloop_rport_lsrqst_work+0x242/0x2e0
>    Workqueue: nvmet-wq fcloop_tport_lsrqst_work
>    Allocated by nvmet_fc_xmt_disconnect_assoc
>    Freed by nvmet_fc_free_pending_reqs
>             nvmet_fc_unregister_targetport
>             fcloop_delete_target_port
> 
> Return -ECONNREFUSED directly instead. __nvmet_fc_send_ls_req() unwinds
> and nvmet_fc_xmt_disconnect_assoc() frees the lsop, so no completion is
> left outstanding.
> 
> Reported-by: syzbot+77955102efac681ec73b@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=77955102efac681ec73b
> Fixes: bbccbf791e6f ("nvmet-fc: free pending reqs on tgtport unregister")
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
>   drivers/nvme/target/fcloop.c | 17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
> index b63af3b643a6..9f9fb40f8a97 100644
> --- a/drivers/nvme/target/fcloop.c
> +++ b/drivers/nvme/target/fcloop.c
> @@ -456,21 +456,18 @@ fcloop_t2h_ls_req(struct nvmet_fc_target_port *targetport, void *hosthandle,
>   	 * 1:1 tgtport vs remoteport
>   	 */
>   
> +	/*
> +	 * Fail synchronously: an async completion can run after
> +	 * nvmet_fc_unregister_targetport() has freed the pending lsreq.
> +	 */
> +	if (!tport->remoteport)
> +		return -ECONNREFUSED;
> +
>   	tls_req = kmem_cache_alloc(lsreq_cache, GFP_KERNEL);
>   	if (!tls_req)
>   		return -ENOMEM;
>   	tls_req->lsreq = lsreq;
>   	INIT_LIST_HEAD(&tls_req->ls_list);
> -
> -	if (!tport->remoteport) {
> -		tls_req->status = -ECONNREFUSED;
> -		spin_lock(&tport->lock);
> -		list_add_tail(&tls_req->ls_list, &tport->ls_list);
> -		spin_unlock(&tport->lock);
> -		queue_work(nvmet_wq, &tport->ls_work);
> -		return ret;
> -	}
> -
>   	tls_req->status = 0;
>   	ret = nvme_fc_rcv_ls_req(tport->remoteport, &tls_req->ls_rsp,
>   				 lsreq->rqstaddr, lsreq->rqstlen);

Can you create a blktest for this?
Otherwise look good.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
[PATCH v2] nvmet-fc: flush nvmet_wq twice on targetport unregister
Posted by Nguyen Ngoc Thang 3 days, 5 hours ago
nvmet_fc_delete_assoc_work() sends a Disconnect Association LS. If the
LLDD completes it asynchronously, e.g. fcloop failing it with
-ECONNREFUSED from a work item once the remote port is gone, the
completion is queued from within nvmet_fc_unregister_targetport()'s
flush_workqueue(). flush_workqueue() does not wait for work queued
while it runs, so nvmet_fc_free_pending_reqs() frees the pending lsop
before the completion runs, which then calls lsreq->done() on freed
memory:

  BUG: KASAN: slab-use-after-free in fcloop_rport_lsrqst_work+0x242/0x2e0
  Workqueue: nvmet-wq fcloop_tport_lsrqst_work
  Allocated by nvmet_fc_xmt_disconnect_assoc
  Freed by nvmet_fc_free_pending_reqs
           nvmet_fc_unregister_targetport
           fcloop_delete_target_port

Flush a second time so such completions have run before the remaining
pending requests are freed.

Reported-by: syzbot+77955102efac681ec73b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=77955102efac681ec73b
Fixes: bbccbf791e6f ("nvmet-fc: free pending reqs on tgtport unregister")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Changes in v2:
- Drop the fcloop change; the async -ECONNREFUSED completion mimics what
  an HBA does (Daniel). Instead flush nvmet_wq a second time in
  nvmet_fc_unregister_targetport() so the LS completion queued by
  nvmet_fc_delete_assoc_work() runs before the pending requests are freed.
- A blktest reproducing this is sent separately (nvme/071) (Hannes).
- v1: https://lore.kernel.org/all/20260920163804.67858-1-ngocthang2710.1999@gmail.com/

 drivers/nvme/target/fc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1b557775e033..cf9fd304a2b5 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1646,6 +1646,8 @@ nvmet_fc_unregister_targetport(struct nvmet_fc_target_port *target_port)
 	/* terminate any outstanding associations */
 	__nvmet_fc_free_assocs(tgtport);
 
+	flush_workqueue(nvmet_wq);
+	/* assoc deletion sends an LS whose completion is queued while flushing */
 	flush_workqueue(nvmet_wq);
 
 	nvmet_fc_free_pending_reqs(tgtport);
-- 
2.43.0