Re: [PATCH 1/5] nvme-fc: don't hold rport lock when putting ctrl

[Hannes Reinecke]

On 10/28/25 16:26, Daniel Wagner wrote:
> nvme_fc_ctrl_put can acquire the rport lock when freeing the
> ctrl object:
>
> nvme_fc_ctrl_put
>    nvme_fc_ctrl_free
>      spin_lock_irqsave(rport->lock)
>
> Thus we can't hold the rport lock when calling nvme_fc_ctrl_put.
>
> Signed-off-by: Daniel Wagner <wagi@xxxxxxxxxx>
> ---
>   drivers/nvme/host/fc.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 03987f497a5b..2c0ea843ae57 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -1488,7 +1488,9 @@ nvme_fc_match_disconn_ls(struct nvme_fc_rport *rport,
>   		if (ret)
>   			/* leave the ctrl get reference */
>   			break;
> +		spin_unlock_irqrestore(&rport->lock, flags);
>   		nvme_fc_ctrl_put(ctrl);
> +		spin_lock_irqsave(&rport->lock, flags);
>   	}
>   
>   	spin_unlock_irqrestore(&rport->lock, flags);
In theory, yes.
In practice we're getting a reference to the controller at the start
of the loop, so nvme_fc_ctrl_put() should just decrement the refcount.
But _if_ someone manages to sneak in an drop the reference while we are
within that loop then the entire logic falls apart as the controller
will be deleted and the next loop iteration will trip over an
uninitialized pointer.
So you would need to modify the loop to use
loop_for_each_entry_safe() to avoid this, too.

Cheers,

Hannes
--
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@xxxxxxx                                +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