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

[Sagi Grimberg]

>> -	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?

OK.

>> -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.

But that is already the case, even if nvme_init_ctrl completes,
it can subsequently fail in nvme_init_ctrl_finish() and the
drivers already simply release the final reference on the
device and go through .release on a partly initialized dev.

> 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.

I don't see how that is possible.

The reason the .release is used in the patch is because device_add
allocates internal resources that are freed when it is released
(see device_private_init called, which is exactly the memleak
addressed in this patch).

So we can't know from nvme what to unwind. The only alternative
I see is to move device_private_init() from device_add to
device_initialize() and introduce device_uninitialize() to
unwind.