Re: unhandled kfree while ramfs_fill_super gets called

[Guo Chao <yan@linux.vnet.ibm.com>]

On Sun, Nov 18, 2012 at 03:47:30AM -0500, devendra.aaru wrote:
> Hi all,
> 
> While looking at the devtmpfs code, i came to know that this callback,
> ramfs_fill_super gets called, i have no idea of what this function does but
> i think i found a leak if any calls after the kzalloc fails we leak
> the fsi element.
> 
> the below patch (build tested) will fix it, but still i am unsure
> whether this is right fix.
> 
> 
> ------------
> Subject: [PATCH] ramfs: kfree the allocated fsi pointer at fail paths in
>  ramfs_fill_super
> 
> while this function gets called from the devtmpfs code,
> 
> the call is from the dev_mount -> if CONFIG_TMPFS disabled,
> mount_single calls the ramfs_fill_super,
> 
> there in this function we allocate the ramfs_fsinfo data structure
> and then call other two functions, but if they fail we forget kfreeing
> the ramfs_fsinfo allocated structure, so free it at the error paths.
> 
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> ---
>  fs/ramfs/inode.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
> index eab8c09..87eb28b 100644
> --- a/fs/ramfs/inode.c
> +++ b/fs/ramfs/inode.c
> @@ -220,8 +220,10 @@ int ramfs_fill_super(struct super_block *sb, void
> *data, int silent)
>                 return -ENOMEM;
> 
>         err = ramfs_parse_options(data, &fsi->mount_opts);
> -       if (err)
> +       if (err) {
> +               kfree(fsi);
>                 return err;
> +       }
> 
>         sb->s_maxbytes          = MAX_LFS_FILESIZE;
>         sb->s_blocksize         = PAGE_CACHE_SIZE;
> @@ -232,8 +234,10 @@ int ramfs_fill_super(struct super_block *sb, void
> *data, int silent)
> 
>         inode = ramfs_get_inode(sb, NULL, S_IFDIR | fsi->mount_opts.mode, 0);
>         sb->s_root = d_make_root(inode);
> -       if (!sb->s_root)
> +       if (!sb->s_root) {
> +               kfree(fsi);
>                 return -ENOMEM;
> +       }
> 
>         return 0;
>  }

If ramfs_fill_super() and ramfs_kill_sb() are used in pair, this memory
will be freed in ramfs_kill_sb(). In some configurations, it apparently
breaks, at least for devtmpfs and tiny-shmem.

As you already free fsi in ramfs_fill_super(), kfree() in
ramfs_kill_sb() can be removed now.

Regards,
Guo Chao