linux-kernel - Re: [PATCH AUTOSEL 4.19 127/206] nvme: Fix controller creation races with teardown flow

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 for Android: free password hash cracker in your pocket

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

Message-ID: <47a9f0da-9fcd-ad17-d2bf-f79767745a39@grimberg.me>
Date:   Fri, 18 Sep 2020 11:46:22 -0700
From:   Sagi Grimberg <sagi@...mberg.me>
To:     Sasha Levin <sashal@...nel.org>, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Cc:     Israel Rukshin <israelr@...lanox.com>,
        linux-nvme@...ts.infradead.org, Keith Busch <kbusch@...nel.org>,
        Max Gurtovoy <maxg@...lanox.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH AUTOSEL 4.19 127/206] nvme: Fix controller creation races
 with teardown flow

This causes a regression and was reverted upstream, just FYI.

On 9/17/20 7:06 PM, Sasha Levin wrote:
> From: Israel Rukshin <israelr@...lanox.com>
> 
> [ Upstream commit ce1518139e6976cf19c133b555083354fdb629b8 ]
> 
> Calling nvme_sysfs_delete() when the controller is in the middle of
> creation may cause several bugs. If the controller is in NEW state we
> remove delete_controller file and don't delete the controller. The user
> will not be able to use nvme disconnect command on that controller again,
> although the controller may be active. Other bugs may happen if the
> controller is in the middle of create_ctrl callback and
> nvme_do_delete_ctrl() starts. For example, freeing I/O tagset at
> nvme_do_delete_ctrl() before it was allocated at create_ctrl callback.
> 
> To fix all those races don't allow the user to delete the controller
> before it was fully created.
> 
> Signed-off-by: Israel Rukshin <israelr@...lanox.com>
> Reviewed-by: Max Gurtovoy <maxg@...lanox.com>
> Reviewed-by: Christoph Hellwig <hch@....de>
> Signed-off-by: Keith Busch <kbusch@...nel.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>   drivers/nvme/host/core.c | 5 +++++
>   drivers/nvme/host/nvme.h | 1 +
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 4b182ac15687e..faa7feebb6095 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2856,6 +2856,10 @@ static ssize_t nvme_sysfs_delete(struct device *dev,
>   {
>   	struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>   
> +	/* Can't delete non-created controllers */
> +	if (!ctrl->created)
> +		return -EBUSY;
> +
>   	if (device_remove_file_self(dev, attr))
>   		nvme_delete_ctrl_sync(ctrl);
>   	return count;
> @@ -3576,6 +3580,7 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl)
>   		queue_work(nvme_wq, &ctrl->async_event_work);
>   		nvme_start_queues(ctrl);
>   	}
> +	ctrl->created = true;
>   }
>   EXPORT_SYMBOL_GPL(nvme_start_ctrl);
>   
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 31c1496f938fb..a70b997060e68 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -206,6 +206,7 @@ struct nvme_ctrl {
>   	struct nvme_command ka_cmd;
>   	struct work_struct fw_act_work;
>   	unsigned long events;
> +	bool created;
>   
>   #ifdef CONFIG_NVME_MULTIPATH
>   	/* asymmetric namespace access: */
>