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

[Sagi Grimberg]

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

On 9/17/20 7:06 PM, Sasha Levin wrote:
> From: Israel Rukshin <israelr@xxxxxxxxxxxx>
>
> [ 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@xxxxxxxxxxxx>
> Reviewed-by: Max Gurtovoy <maxg@xxxxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Keith Busch <kbusch@xxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
>   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: */