Re: [PATCH v2] 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 3bf5702b60af10d1adf6131dcac2e5211f8bdb47
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 patch skips the entry if it is already
bound to prevent clobbering existing bindings.

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 target port reference?

The commit message states "Fix this by checking if pe->tgtport is already set
before acquiring a reference count", but the check is placed immediately after
nvmet_fc_tgtport_get() rather than before it.

If pe->tgtport is already bound, the continue statement will skip the rest of
the loop, leaving the reference count acquired by nvmet_fc_tgtport_get()
permanently elevated.

Could the check be moved before nvmet_fc_tgtport_get() as intended?

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

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