From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guo Chao Subject: Re: unhandled kfree while ramfs_fill_super gets called Date: Mon, 19 Nov 2012 15:22:16 +0800 Message-ID: <20121119072216.GA3703@yanx> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org To: "devendra.aaru" Return-path: Received: from e28smtp02.in.ibm.com ([122.248.162.2]:51016 "EHLO e28smtp02.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751248Ab2KSHWk (ORCPT ); Mon, 19 Nov 2012 02:22:40 -0500 Received: from /spool/local by e28smtp02.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 19 Nov 2012 12:52:37 +0530 Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay05.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qAJ7MYkI55509220 for ; Mon, 19 Nov 2012 12:52:35 +0530 Received: from d28av04.in.ibm.com (loopback [127.0.0.1]) by d28av04.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qAJCqPId028359 for ; Mon, 19 Nov 2012 23:52:25 +1100 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > --- > 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