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

[sashiko-bot@kernel.org]

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