unhandled kfree while ramfs_fill_super gets called

unhandled kfree while ramfs_fill_super gets called

[devendra.aaru]

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;
 }
--
1.7.10.4

Re: unhandled kfree while ramfs_fill_super gets called

[Guo Chao]

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

Re: unhandled kfree while ramfs_fill_super gets called

[devendra.aaru]

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

i dunno about this, thanks for telling me :)
i am not at this stage of fixing those bugs as i dont have any further
knowledge of the fs calls and functions.

so i cant be fixing this your way, anyways i am dropping my patch now :)