All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvmet-fc: skip hostport ops when the LLDD has no host handle
@ 2026-10-14  7:53 kensanya
  2026-10-15  6:14 ` Christoph Hellwig
  0 siblings, 1 reply; 3+ messages in thread
From: kensanya @ 2026-10-14  7:53 UTC (permalink / raw)
  To: justin.tee, nareshgottumukkala83, paul.ely, hch, sagi, kch
  Cc: linux-nvme, linux-kernel, TanZheng

From: TanZheng <tanzheng@kylinos.cn>

nvmet_fc_alloc_hostport() returns NULL when the LLDD passes a NULL
hosthandle. That is not an ERR_PTR, so nvmet_fc_alloc_target_assoc()
keeps the association and leaves assoc->hostport NULL.

Disconnect, association free and host invalidation then dereference
assoc->hostport. The disconnect comment already says an older LLDD
with no host handle should not send an LS.

Treat a missing hostport like a missing ls_req: do not send the
disconnect LS, do not put the hostport, and skip that association in
nvmet_fc_invalidate_host().

Assisted-by: Kimi:K3
Signed-off-by: TanZheng <tanzheng@kylinos.cn>
---
 drivers/nvme/target/fc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1b557775e033..951be0bf7b54 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -483,7 +483,8 @@ nvmet_fc_xmt_disconnect_assoc(struct nvmet_fc_tgt_assoc *assoc)
 	 * message is normal. Otherwise, send unless the hostport has
 	 * already been invalidated by the lldd.
 	 */
-	if (!tgtport->ops->ls_req || assoc->hostport->invalid)
+	if (!tgtport->ops->ls_req || !assoc->hostport ||
+	    assoc->hostport->invalid)
 		return;
 
 	lsop = kzalloc((sizeof(*lsop) +
@@ -1176,7 +1177,8 @@ nvmet_fc_target_assoc_free(struct kref *ref)
 	/* Send Disconnect now that all i/o has completed */
 	nvmet_fc_xmt_disconnect_assoc(assoc);
 
-	nvmet_fc_hostport_put(assoc->hostport);
+	if (assoc->hostport)
+		nvmet_fc_hostport_put(assoc->hostport);
 	spin_lock_irqsave(&tgtport->lock, flags);
 	oldls = assoc->rcv_disconn;
 	spin_unlock_irqrestore(&tgtport->lock, flags);
@@ -1526,7 +1528,8 @@ nvmet_fc_invalidate_host(struct nvmet_fc_target_port *target_port,
 	spin_lock_irqsave(&tgtport->lock, flags);
 	list_for_each_entry_safe(assoc, next,
 				&tgtport->assoc_list, a_list) {
-		if (assoc->hostport->hosthandle != hosthandle)
+		if (!assoc->hostport ||
+		    assoc->hostport->hosthandle != hosthandle)
 			continue;
 		if (!nvmet_fc_tgt_a_get(assoc))
 			continue;



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] nvmet-fc: skip hostport ops when the LLDD has no host handle
  2026-10-14  7:53 [PATCH] nvmet-fc: skip hostport ops when the LLDD has no host handle kensanya
@ 2026-10-15  6:14 ` Christoph Hellwig
  2026-10-15 11:20   ` kensanya
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2026-10-15  6:14 UTC (permalink / raw)
  To: kensanya
  Cc: justin.tee, nareshgottumukkala83, paul.ely, hch, sagi, kch,
	linux-nvme, linux-kernel, TanZheng

On Wed, Oct 14, 2026 at 03:53:03PM +0800, kensanya@163.com wrote:
> From: TanZheng <tanzheng@kylinos.cn>
> 
> nvmet_fc_alloc_hostport() returns NULL when the LLDD passes a NULL
> hosthandle. That is not an ERR_PTR, so nvmet_fc_alloc_target_assoc()
> keeps the association and leaves assoc->hostport NULL.

A NULL hosthandle doesn't make much sense, so why do you want to handle
that?



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re:Re: [PATCH] nvmet-fc: skip hostport ops when the LLDD has no host handle
  2026-10-15  6:14 ` Christoph Hellwig
@ 2026-10-15 11:20   ` kensanya
  0 siblings, 0 replies; 3+ messages in thread
From: kensanya @ 2026-10-15 11:20 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: justin.tee, nareshgottumukkala83, paul.ely, sagi, kch, linux-nvme,
	linux-kernel, TanZheng

At 2026-10-15 14:14:49, "Christoph Hellwig" <hch@lst.de> wrote:
>On Wed, Oct 14, 2026 at 03:53:03PM +0800, kensanya@163.com wrote:
>> From: TanZheng <tanzheng@kylinos.cn>
>> 
>> nvmet_fc_alloc_hostport() returns NULL when the LLDD passes a NULL
>> hosthandle. That is not an ERR_PTR, so nvmet_fc_alloc_target_assoc()
>> keeps the association and leaves assoc->hostport NULL.
>
>A NULL hosthandle doesn't make much sense, so why do you want to handle
>that?

Hi.

nvmet_fc_rcv_ls_req() only stores the hosthandle in the iod. It does not call 
nvmet_fc_alloc_hostport(). That function returns NULL for a NULL hosthandle, 
and nvmet_fc_alloc_target_assoc() stores it in assoc->hostport. 
The association is still created, so teardown can NULL-dereference it in nvmet_
fc_hostport_put(), and in nvmet_fc_xmt_disconnect_assoc() when ops->ls_req is set.

nvmet_fc_hostport_put() needs the same check. No hostport is allocated and 
no kref is taken, but nvmet_fc_target_assoc_free() still calls nvmet_fc_hostport_put(assoc->hostport), 
which does kref_put(&hostport->ref, nvmet_fc_hostport_free).

Best regard.
TanZheng.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-15 11:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14  7:53 [PATCH] nvmet-fc: skip hostport ops when the LLDD has no host handle kensanya
2026-10-15  6:14 ` Christoph Hellwig
2026-10-15 11:20   ` kensanya

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.