Re: [PATCH] reiserfs: Fix locking BUG during mount failure

[Frederic Weisbecker <fweisbec@gmail.com>]

On Mon, Mar 29, 2010 at 03:12:39PM -0400, Jeff Mahoney wrote:
>  Commit 8ebc423238341b52912c7295b045a32477b33f09 (reiserfs: kill-the-BKL)
>  introduced a bug in the mount failure case.
> 
>  The error label releases the lock before calling journal_release_error,
>  but it requires that the lock be held. do_journal_release unlocks and
>  retakes it. When it releases it without it held, we trigger a BUG().
> 
>  The error_alloc label skips the unlock since the lock isn't held yet 
>  but none of the other conditions that are clean up exist yet either.
> 
>  This patch returns immediately after the kzalloc failure and moves
>  the reiserfs_write_unlock after the journal_release_error call.
> 
>  This was reported in https://bugzilla.novell.com/show_bug.cgi?id=591807
> 
> Reported-by:  Thomas Siedentopf <thomas.siedentopf@novell.com>
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>


I've applied it. Thanks a lot!



> ---
>  fs/reiserfs/super.c |   10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> --- a/fs/reiserfs/super.c
> +++ b/fs/reiserfs/super.c
> @@ -1618,10 +1618,8 @@ static int reiserfs_fill_super(struct su
>  	save_mount_options(s, data);
>  
>  	sbi = kzalloc(sizeof(struct reiserfs_sb_info), GFP_KERNEL);
> -	if (!sbi) {
> -		errval = -ENOMEM;
> -		goto error_alloc;
> -	}
> +	if (!sbi)
> +		return -ENOMEM;
>  	s->s_fs_info = sbi;
>  	/* Set default values for options: non-aggressive tails, RO on errors */
>  	REISERFS_SB(s)->s_mount_opt |= (1 << REISERFS_SMALLTAIL);
> @@ -1881,12 +1879,12 @@ static int reiserfs_fill_super(struct su
>  	return (0);
>  
>  error:
> -	reiserfs_write_unlock(s);
> -error_alloc:
>  	if (jinit_done) {	/* kill the commit thread, free journal ram */
>  		journal_release_error(NULL, s);
>  	}
>  
> +	reiserfs_write_unlock(s);
> +
>  	reiserfs_free_bitmap_cache(s);
>  	if (SB_BUFFER_WITH_SB(s))
>  		brelse(SB_BUFFER_WITH_SB(s));