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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <accf6ad7-047b-48d0-b626-f3ef0facd649@suse.de>
Date: Tue, 4 Nov 2025 11:51:09 +0100
From: Hannes Reinecke <hare@...e.de>
To: Daniel Wagner <wagi@...nel.org>, Justin Tee <justin.tee@...adcom.com>,
 Christoph Hellwig <hch@....de>, Keith Busch <kbusch@...nel.org>
Cc: James Smart <james.smart@...adcom.com>, Jens Axboe <axboe@...nel.dk>,
 linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] nvme-fc: don't hold rport lock when putting ctrl

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@...nel.org>
> ---
>   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@...e.de                                +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