Re: [PATCH] nvme: Fix memory leak in nvme_init_ctrl error path

[Christoph Hellwig <hch@lst.de>]

On Wed, May 03, 2023 at 06:09:25PM +0300, Sagi Grimberg wrote:
> -	if (!ctrl->discard_page) {
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> +	if (!ctrl->discard_page)
> +		return -ENOMEM;

Can we please pre-load these cleanups in a separate patch?

> -out:
> -	if (ctrl->discard_page)
> -		__free_page(ctrl->discard_page);
> +	ctrl->instance = NVME_CTRL_INSTANCE_UNINITIALIZED;
> +	/* pairs with device_initialize .release method will cleanup */
> +	nvme_put_ctrl(ctrl);

Err, no.  We should not go through .release with a partial 
initialization.  Please do proper unwinding before the device
is added, and make sure everything is in a proper state by the
time ->release can be called.

>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(nvme_init_ctrl);
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index bf46f122e9e1..920403589670 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -255,6 +255,7 @@ struct nvme_ctrl {
>  	struct request_queue *connect_q;
>  	struct request_queue *fabrics_q;
>  	struct device *dev;
> +#define NVME_CTRL_INSTANCE_UNINITIALIZED (-1)
>  	int instance;
>  	int numa_node;
>  	struct blk_mq_tag_set *tagset;
> -- 
> 2.34.1
---end quoted text---