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: <aBEW4W40ZelIXfs2@kbusch-mbp>
Date: Tue, 29 Apr 2025 11:13:53 -0700
From: Keith Busch <kbusch@...nel.org>
To: Hannes Reinecke <hare@...e.de>
Cc: Daniel Wagner <dwagner@...e.de>, Guenter Roeck <linux@...ck-us.net>,
	Daniel Wagner <wagi@...nel.org>, Jens Axboe <axboe@...nel.dk>,
	Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
	James Smart <james.smart@...adcom.com>,
	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

On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote:
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index b502ac07483b..d3c4eacf607f 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work)
> >                  msleep(100);
> >          }
> > 
> > -       if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> > +       if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
> > +           !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> >                  return;
> > 
> >          nvme_unquiesce_io_queues(ctrl);
> 
> I would rather have a separate state for firmware activation.
> (Ab-)using the 'RESETTING' state here has direct implications
> with the error handler, as for the error handler 'RESETTING'
> means that the error handler has been scheduled.
> Which is not true for firmware activation.

But the point of having firmware activation set the state to RESETTING
was to fence off error handling from trying to schedule a real reset.
The fw activation work schedules its own recovery if it times out, but
we don't want any other recovery action or user requested resets to
proceed while an activation is still pending.