Re: [PATCH 1/9] md: fix error handling in md_alloc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2022-07-13 05:31, Christoph Hellwig wrote:
> Error handling in md_alloc is a mess.  Untangle it to just free the mddev
> directly before add_disk is called and thus the gendisk is globally
> visible.  After that clear the hold flag and let the mddev_put take care
> of cleaning up the mddev through the usual mechanisms.
> 
> Fixes: 5e55e2f5fc95 ("[PATCH] md: convert compile time warnings into runtime warnings")
> Fixes: 9be68dd7ac0e ("md: add error handling support for add_disk()")
> Fixes: 7ad1069166c0 ("md: properly unwind when failing to add the kobject in md_alloc")
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> 
> Note: the put_disk here is not fully correct on md-next, but will
> do the right thing once merged with the block tree.
> 
>  drivers/md/md.c | 45 ++++++++++++++++++++++++++++++++-------------
>  1 file changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index b64de313838f2..7affddade8b6b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -791,6 +791,15 @@ static struct mddev *mddev_alloc(dev_t unit)
>  	return ERR_PTR(error);
>  }
>  
> +static void mddev_free(struct mddev *mddev)
> +{
> +	spin_lock(&all_mddevs_lock);
> +	list_del(&mddev->all_mddevs);
> +	spin_unlock(&all_mddevs_lock);
> +
> +	kfree(mddev);
> +}

Sadly, I still don't think this is correct. mddev_init() has already
called kobject_init() and according to the documentation it *must* be
cleaned up with a call to kobject_put(). That doesn't happen in this
path -- so I believe this will cause memory leaks.

I have also noticed this code base already makes that same mistake in
mddev_alloc(): freeing the mddev on the error path after kobject_init().
But that would be easy to fix up.

Logan



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux