[PATCH] reiserfs: Fix locking BUG during mount failure

[PATCH] reiserfs: Fix locking BUG during mount failure

[Jeff Mahoney]

 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>
---
 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));

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

[Frederic Weisbecker]

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));