linux-kernel - Re: [PATCH 1/2] nvme: only allow entering LIVE from CONNECTING state

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: <0134ea15-8d5f-41f7-9e9a-d7e6d82accaa@roeck-us.net>
Date: Sun, 27 Apr 2025 08:59:13 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Daniel Wagner <wagi@...nel.org>
Cc: Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...nel.dk>,
	Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
	James Smart <james.smart@...adcom.com>,
	Hannes Reinecke <hare@...e.de>,
	Shinichiro Kawasaki <shinichiro.kawasaki@....com>,
	linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] nvme: only allow entering LIVE from CONNECTING state

Hi,

On Fri, Feb 14, 2025 at 09:02:03AM +0100, Daniel Wagner wrote:
> The fabric transports and also the PCI transport are not entering the
> LIVE state from NEW or RESETTING. This makes the state machine more
> restrictive and allows to catch not supported state transitions, e.g.
> directly switching from RESETTING to LIVE.
> 
> Signed-off-by: Daniel Wagner <wagi@...nel.org>

nvme_handle_aen_notice(), when handling NVME_AER_NOTICE_FW_ACT_STARTING,
sets the state to RESETTING and and triggers a worker. This worker
waits for firmware activation to complete and then tries to set the
state back to LIVE. This step now fails.

Possibly the handling of NVME_AER_NOTICE_FW_ACT_STARTING needs to be
improved. However, leaving the NVME in RESETTING state after an
NVME_AER_NOTICE_FW_ACT_STARTING event is worse.

I think this patch should be reverted at least for the time being until
the handling of NVME_AER_NOTICE_FW_ACT_STARTING no longer relies on a
direct state change from RESETTING to LIVE.

Please correct me if my analysis is wrong.

Thanks,
Guenter

> ---
>  drivers/nvme/host/core.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 818d4e49aab51c388af9a48bf9d466fea9cef51b..f028913e2e622ee348e88879c6e6b7e8f8a1cc82 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -564,8 +564,6 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
>  	switch (new_state) {
>  	case NVME_CTRL_LIVE:
>  		switch (old_state) {
> -		case NVME_CTRL_NEW:
> -		case NVME_CTRL_RESETTING:
>  		case NVME_CTRL_CONNECTING:
>  			changed = true;
>  			fallthrough;
> 
> -- 
> 2.48.1
>