[PATCH] omfs: fix memory leak
in [Kernel]
|
From: Davidlohr Bueso on 3 Jul 2010 22:40 Hi, In omfs_fill_super(), when returning on error, sbi is not being freed. Thanks, Davidlohr. Signed-off-by: Davidlohr Bueso <dave(a)gnu.org> --- fs/omfs/inode.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/fs/omfs/inode.c b/fs/omfs/inode.c index 089839a..253846e 100644 --- a/fs/omfs/inode.c +++ b/fs/omfs/inode.c @@ -523,12 +523,14 @@ static int omfs_fill_super(struct super_block *sb, void *data, int silent) } printk(KERN_DEBUG "omfs: Mounted volume %s\n", omfs_rb->r_name); - ret = 0; + ret = 0; /* success */ out_brelse_bh2: brelse(bh2); out_brelse_bh: brelse(bh); end: + if (ret != 0) + kfree(sbi); return ret; } -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: me on 5 Jul 2010 10:10 On Mon, Jul 05, 2010 at 01:12:39AM -0400, Davidlohr Bueso wrote: > Isn't put_super() called to free data when things run "normally", like > for unmounting? So this function does two things: Ok, I checked it out and you are right, FS put_super is only called after successful mount so there is a leak. I'll take your patch, but please: - remove the /* success */ comment, IMO it's just noise - write the if conditional as the more usual: if (ret) > kfree(sbi->s_imap) > kfree(sbi) > > However, in omfs_get_imap() 'sbi->s_imap' is freed upon failure, so > wouldn't this also crash on the first kfree in omfs_put_super()? This is ok, since sbi->s_imap is set to null in that case and kfree(NULL) is fine. Thanks for the review! -- Bob Copeland %% www.bobcopeland.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Davidlohr Bueso on 6 Jul 2010 01:00 On Mon, 2010-07-05 at 10:00 -0400, me(a)bobcopeland.com wrote: > On Mon, Jul 05, 2010 at 01:12:39AM -0400, Davidlohr Bueso wrote: > > Isn't put_super() called to free data when things run "normally", like > > for unmounting? So this function does two things: > > Ok, I checked it out and you are right, FS put_super is only called > after successful mount so there is a leak. I'll take your patch, > but please: > > - remove the /* success */ comment, IMO it's just noise > - write the if conditional as the more usual: > if (ret) > Ok, resending that patch with the corrections. Thanks. Signed-off-by: Davidlohr Bueso <dave(a)gnu.org> --- fs/omfs/inode.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/fs/omfs/inode.c b/fs/omfs/inode.c index 089839a..b5d6380 100644 --- a/fs/omfs/inode.c +++ b/fs/omfs/inode.c @@ -529,6 +529,8 @@ out_brelse_bh2: out_brelse_bh: brelse(bh); end: + if (ret) + kfree(sbi); return ret; } -- 1.7.0.4 > > kfree(sbi->s_imap) > > kfree(sbi) > > > > However, in omfs_get_imap() 'sbi->s_imap' is freed upon failure, so > > wouldn't this also crash on the first kfree in omfs_put_super()? > > This is ok, since sbi->s_imap is set to null in that case and > kfree(NULL) is fine. > > Thanks for the review! > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: me on 6 Jul 2010 11:50 On Tue, Jul 06, 2010 at 12:50:58AM -0400, Davidlohr Bueso wrote: > Ok, resending that patch with the corrections. Thanks. > > Signed-off-by: Davidlohr Bueso <dave(a)gnu.org> Great, applied to the fixes branch of git://git.kernel.org/pub/scm/linux/kernel/git/bcopeland/omfs.git Thanks! -- Bob Copeland %% www.bobcopeland.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|
Pages: 1 Prev: EXT3 FS and 64K blocks error Next: VIDEO: ivtvfb, remove unneeded NULL test |