From: me@bobcopeland.com
To: Davidlohr Bueso <dave.bueso@gmail.com>
Cc: linux-fsdevel@vger.kernel.org,
linux-karma-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org
Subject: Re: [linux-karma-devel] [PATCH] omfs: fix memory leak
Date: Mon, 5 Jul 2010 10:00:08 -0400 [thread overview]
Message-ID: <20100705140008.GA3378@hash.localnet> (raw)
In-Reply-To: <1278306759.3811.24.camel@cowboy>
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
WARNING: multiple messages have this Message-ID (diff)
From: me@bobcopeland.com
To: Davidlohr Bueso <dave.bueso@gmail.com>
Cc: linux-fsdevel@vger.kernel.org,
linux-karma-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] omfs: fix memory leak
Date: Mon, 5 Jul 2010 10:00:08 -0400 [thread overview]
Message-ID: <20100705140008.GA3378@hash.localnet> (raw)
In-Reply-To: <1278306759.3811.24.camel@cowboy>
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
------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
next prev parent reply other threads:[~2010-07-05 14:02 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-04 2:33 [PATCH] omfs: fix memory leak Davidlohr Bueso
2010-07-04 2:33 ` Davidlohr Bueso
2010-07-04 11:37 ` me
2010-07-05 5:12 ` Davidlohr Bueso
2010-07-05 5:12 ` Davidlohr Bueso
2010-07-05 14:00 ` me [this message]
2010-07-05 14:00 ` me
2010-07-06 4:50 ` [linux-karma-devel] " Davidlohr Bueso
2010-07-06 15:45 ` me
2010-07-06 15:45 ` me
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100705140008.GA3378@hash.localnet \
--to=me@bobcopeland.com \
--cc=dave.bueso@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-karma-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.