From mboxrd@z Thu Jan 1 00:00:00 1970 From: me@bobcopeland.com Subject: Re: [PATCH] omfs: fix memory leak Date: Mon, 5 Jul 2010 10:00:08 -0400 Message-ID: <20100705140008.GA3378@hash.localnet> References: <1278210828.27014.2.camel@cowboy> <20100704113711.GA15067@hash.localnet> <1278306759.3811.24.camel@cowboy> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: linux-fsdevel@vger.kernel.org, linux-karma-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org To: Davidlohr Bueso Return-path: Content-Disposition: inline In-Reply-To: <1278306759.3811.24.camel@cowboy> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-karma-devel-bounces@lists.sourceforge.net List-Id: linux-fsdevel.vger.kernel.org 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