On Sun, 2010-07-04 at 07:37 -0400, me@xxxxxxxxxxxxxxx wrote: > On Sat, Jul 03, 2010 at 10:33:48PM -0400, Davidlohr Bueso wrote: > > Hi, > > > > In omfs_fill_super(), when returning on error, sbi is not being freed. > > > > Thanks, > > Davidlohr. > > Hi Davidlohr, > > I don't think this is right: > > fill_super: > err = omfs_fill_super() > if (err) > deactivate_locked_super(sb) > kill_sb() > generic_shutdown_super() > sop->put_super() > ... > omfs_put_super() > kfree(sbi->s_imap); > kfree(sbi); > > So your change would cause a crash at the first kfree in omfs_put_super(). > > It looks fine to me as-is, or am I missing something? Thanks for the reply, Bob. Please bare with me, as I am learning about the VFS workings. To my knowledge this would happen: a user tries to mount a omfs partition: omfs_get_sb() -> omfs_fill_super() If omfs_fill_super() fails, in this case 'ret' being a non 0 value, the mount will not be completed, hence the previously allocated 'sbi' variable will not be freed. Isn't put_super() called to free data when things run "normally", like for unmounting? So this function does two things: 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()? Wouldn't this be the same for freeing sbi upon failure in omfs_fill_super()? Now, the patch I sent is compile-tested only, so it might crash it. I will test it properly. If what I just said is complete nonsense, please enlighten me. Thanks, Davidlohr > > > > > Signed-off-by: Davidlohr Bueso <dave@xxxxxxx> > > --- > > 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; > > } > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
