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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 06 Jul 2010 00:50:58 -0400
From:	Davidlohr Bueso <dave.bueso@...il.com>
To:	me@...copeland.com
Cc:	linux-fsdevel@...r.kernel.org,
	linux-karma-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [linux-karma-devel] [PATCH] omfs: fix memory leak

On Mon, 2010-07-05 at 10:00 -0400, me@...copeland.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@....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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/