linux-kernel - Re: [PATCH v3 2/2] nvme: handle persistent internal error AER from NVMe controller

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: <20220607103538.GA25071@lst.de>
Date:   Tue, 7 Jun 2022 12:35:38 +0200
From:   Christoph Hellwig <hch@....de>
To:     Michael Kelley <mikelley@...rosoft.com>
Cc:     kbusch@...nel.org, axboe@...com, hch@....de, sagi@...mberg.me,
        linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org,
        caroline.subramoney@...rosoft.com, riwurd@...rosoft.com,
        nathan.obr@...rosoft.com
Subject: Re: [PATCH v3 2/2] nvme: handle persistent internal error AER from
 NVMe controller

On Mon, Jun 06, 2022 at 05:15:15PM -0700, Michael Kelley wrote:
> +static void nvme_handle_aer_persistent_error(struct nvme_ctrl *ctrl)
> +{
> +	trace_nvme_async_event(ctrl, NVME_AER_ERROR);
> +
> +	/*
> +	 * We can't read the CSTS here because we're in an atomic context on
> +	 * some transports and the read may require submitting a request to the
> +	 * to the controller and getting a response. Such a sequence isn't
> +	 * likely to be successful anyway if the controller is reporting a
> +	 * persistent internal error. So assume CSTS.CFS is set.
> +	 */
> +	if (nvme_should_reset(ctrl, NVME_CSTS_CFS)) {
> +		dev_warn(ctrl->device, "resetting controller due to AER\n");
> +		nvme_reset_ctrl(ctrl);

I don't think we even need the nvme_should_reset check now.

nvme_reset_ctrl first calls nvme_change_ctrl_state, which only allows
the transition to the RESETTING state if it previously was NEW or LIVE,
so we are already covered.  The only downside would be an extra kernel
message if we already were in another state.