Re: [linux-karma-devel] [PATCH] omfs: fix memory leak

[Davidlohr Bueso]

On Mon, 2010-07-05 at 10:00 -0400, me@xxxxxxxxxxxxxxx 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@xxxxxxx>

---
 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/