[PATCH] nvmet-fc: skip port entry rebinding if already bound to a target port

[PATCH] nvmet-fc: skip port entry rebinding if already bound to a target port

[Nguyen Duy Nhat Anh]

In nvmet_fc_portentry_rebind_tgt(), nvmet_fc_tgtlock serializes access to
nvmet_fc_portentry_list, but does not prevent multiple concurrent or
duplicate target port registrations from attempting to rebind to an
already-assigned port entry.

When pe->tgtport is non-NULL, WARN_ON() is triggered and pe->tgtport is
overwritten, leading to kernel warnings and leaking the reference count
acquired by nvmet_fc_tgtport_get().

Fix this by checking if pe->tgtport is already set before acquiring a
reference count and rebinding. If it is bound, skip the entry to prevent
clobbering existing bindings.

Reported-by: syzbot+be62d4b375542ecebd17@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=be62d4b375542ecebd17
Signed-off-by: Nguyen Duy Nhat Anh <neganhat@gmail.com>
---
 drivers/nvme/target/fc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1b557775e033..9b7fd8f5bd47 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1324,7 +1324,9 @@ nvmet_fc_portentry_rebind_tgt(struct nvmet_fc_tgtport *tgtport)
 			if (!nvmet_fc_tgtport_get(tgtport))
 				continue;
 
-			WARN_ON(pe->tgtport);
+			if (pe->tgtport)
+				continue;
+
 			tgtport->pe = pe;
 			pe->tgtport = tgtport;
 			break;
-- 
2.55.0

Re: [PATCH] nvmet-fc: skip port entry rebinding if already bound to a target port

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nvme-fc: refcount leak of tgtport in nvmet_fc_portentry_rebind_tgt()
--

commit 3f22f46c30c7c3045ffe22fb5654e54b5aabd759
Author: Nguyen Duy Nhat Anh <neganhat@gmail.com>

nvmet-fc: skip port entry rebinding if already bound to a target port

In nvmet_fc_portentry_rebind_tgt(), nvmet_fc_tgtlock serializes access to
nvmet_fc_portentry_list, but does not prevent multiple concurrent or
duplicate target port registrations from attempting to rebind to an
already-assigned port entry. This fix checks if pe->tgtport is already
set before rebinding to prevent clobbering existing bindings.

Closes: https://syzkaller.appspot.com/bug?extid=be62d4b375542ecebd17

> Fix this by checking if pe->tgtport is already set before acquiring a
> reference count and rebinding. If it is bound, skip the entry to prevent
> clobbering existing bindings.

[Severity: Medium]
The commit message states the check is performed before acquiring the
reference count, but the diff below shows the check occurring after
nvmet_fc_tgtport_get() is called.

> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index eecdffefa24fa..ac3194497a1e0 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -1324,7 +1324,9 @@ nvmet_fc_portentry_rebind_tgt(struct nvmet_fc_tgtport *tgtport)
>  			if (!nvmet_fc_tgtport_get(tgtport))
>  				continue;
>  
> -			WARN_ON(pe->tgtport);
> +			if (pe->tgtport)
> +				continue;
> +

[Severity: Medium]
Does this code leak the tgtport reference?

If pe->tgtport is already set, this continue path skips the entry but doesn't
drop the reference acquired by nvmet_fc_tgtport_get(tgtport) just above it.

Should the check for pe->tgtport be moved before the nvmet_fc_tgtport_get()
call, or should it call nvmet_fc_tgtport_put() before continuing?

>  			tgtport->pe = pe;
>  			pe->tgtport = tgtport;
>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016154807.9544-1-neganhat@gmail.com?part=1

[PATCH] nvmet-fc: skip port entry rebinding if already bound to a target port

[Nguyen Duy Nhat Anh]

In nvmet_fc_portentry_rebind_tgt(), nvmet_fc_tgtlock serializes access to
nvmet_fc_portentry_list, but does not prevent multiple concurrent or
duplicate target port registrations from attempting to rebind to an
already-assigned port entry.

When pe->tgtport is non-NULL, WARN_ON() is triggered and pe->tgtport is
overwritten, leading to kernel warnings and leaking the reference count
acquired by nvmet_fc_tgtport_get().

Fix this by checking if pe->tgtport is already set before acquiring a
reference count and rebinding. If it is bound, skip the entry to prevent
clobbering existing bindings.

Reported-by: syzbot+be62d4b375542ecebd17@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=be62d4b375542ecebd17
Signed-off-by: Nguyen Duy Nhat Anh <neganhat@gmail.com>
---
 drivers/nvme/target/fc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1b557775e033..9b7fd8f5bd47 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -1324,7 +1324,9 @@ nvmet_fc_portentry_rebind_tgt(struct nvmet_fc_tgtport *tgtport)
 			if (!nvmet_fc_tgtport_get(tgtport))
 				continue;
 
-			WARN_ON(pe->tgtport);
+			if (pe->tgtport)
+				continue;
+
 			tgtport->pe = pe;
 			pe->tgtport = tgtport;
 			break;
-- 
2.55.0

Re: [PATCH] nvmet-fc: skip port entry rebinding if already bound to a target port

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nvme-target/fc: reference count leak of tgtport on rebind conflict
--

commit 69be6b42e2f6eef3ebd6d221309e5ccb64969922
Author: Nguyen Duy Nhat Anh <neganhat@gmail.com>

nvmet-fc: skip port entry rebinding if already bound to a target port

This commit adds a check to avoid clobbering existing target port
assignments during rebinding. It prevents multiple concurrent or duplicate
target port registrations from overwriting an already-assigned port entry.

Closes: https://syzkaller.appspot.com/bug?extid=be62d4b375542ecebd17

> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index eecdffefa24fa..ac3194497a1e0 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -1324,7 +1324,9 @@ nvmet_fc_portentry_rebind_tgt(struct nvmet_fc_tgtport *tgtport)
>  			if (!nvmet_fc_tgtport_get(tgtport))
>  				continue;
>  
> -			WARN_ON(pe->tgtport);
> +			if (pe->tgtport)
> +				continue;
> +

[Severity: Medium]
Does this code leak the reference count on the target port?

The commit message states the intent to check for an existing assignment
before acquiring the reference count, but the check is placed immediately
after nvmet_fc_tgtport_get() has already incremented the reference. 

If pe->tgtport is already set, the continue statement will move to the next
iteration without releasing the reference acquired by nvmet_fc_tgtport_get(),
permanently leaking it and preventing the target port from being freed upon
unregistration.

>  			tgtport->pe = pe;
>  			pe->tgtport = tgtport;
>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016155415.10521-1-neganhat@gmail.com?part=1