Patchwork ext4: Fix possible use after free of buffer head

login
register
mail settings
Submitter Lukas Czerner
Date Nov. 29, 2012, 10:43 a.m.
Message ID <1354185828-28545-1-git-send-email-lczerner@redhat.com>
Download mbox | patch
Permalink /patch/202701/
State Accepted, archived
Headers show

Comments

Lukas Czerner - Nov. 29, 2012, 10:43 a.m.
Commit fa77dcfafeaa6bc73293c646bfc3d5192dcf0be2 introduces block bitmap
checksum calculation into ext4_new_inode() in the case that block group
was uninitialized. However we brelse() the bitmap buffer before we
attempt to checksum it so we have no guarantee that the buffer is still
there.

Fix this by releasing the buffer after the possible checksum
computation.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Cc: Darrick J. Wong <djwong@us.ibm.com>
Cc: stable@vger.kernel.org
---
 fs/ext4/ialloc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Darrick J. Wong - Nov. 29, 2012, 7:02 p.m.
On Thu, Nov 29, 2012 at 11:43:48AM +0100, Lukas Czerner wrote:
> Commit fa77dcfafeaa6bc73293c646bfc3d5192dcf0be2 introduces block bitmap
> checksum calculation into ext4_new_inode() in the case that block group
> was uninitialized. However we brelse() the bitmap buffer before we
> attempt to checksum it so we have no guarantee that the buffer is still
> there.
> 
> Fix this by releasing the buffer after the possible checksum
> computation.

Looks ok, so:
Acked-by: Darrick J. Wong <darrick.wong@oracle.com>

That IBM fellow is gone. ;)

--D
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Cc: Darrick J. Wong <djwong@us.ibm.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/ext4/ialloc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 3a100e7..c7efa88 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -762,7 +762,6 @@ got:
>  
>  		BUFFER_TRACE(block_bitmap_bh, "dirty block bitmap");
>  		err = ext4_handle_dirty_metadata(handle, NULL, block_bitmap_bh);
> -		brelse(block_bitmap_bh);
>  
>  		/* recheck and clear flag under lock if we still need to */
>  		ext4_lock_group(sb, group);
> @@ -775,6 +774,7 @@ got:
>  			ext4_group_desc_csum_set(sb, group, gdp);
>  		}
>  		ext4_unlock_group(sb, group);
> +		brelse(block_bitmap_bh);
>  
>  		if (err)
>  			goto fail;
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o - Nov. 30, 2012, 2:21 a.m.
On Thu, Nov 29, 2012 at 11:02:39AM -0800, Darrick J. Wong wrote:
> On Thu, Nov 29, 2012 at 11:43:48AM +0100, Lukas Czerner wrote:
> > Commit fa77dcfafeaa6bc73293c646bfc3d5192dcf0be2 introduces block bitmap
> > checksum calculation into ext4_new_inode() in the case that block group
> > was uninitialized. However we brelse() the bitmap buffer before we
> > attempt to checksum it so we have no guarantee that the buffer is still
> > there.
> > 
> > Fix this by releasing the buffer after the possible checksum
> > computation.
> 
> Looks ok, so:
> Acked-by: Darrick J. Wong <darrick.wong@oracle.com>

Applied, thanks.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 3a100e7..c7efa88 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -762,7 +762,6 @@  got:
 
 		BUFFER_TRACE(block_bitmap_bh, "dirty block bitmap");
 		err = ext4_handle_dirty_metadata(handle, NULL, block_bitmap_bh);
-		brelse(block_bitmap_bh);
 
 		/* recheck and clear flag under lock if we still need to */
 		ext4_lock_group(sb, group);
@@ -775,6 +774,7 @@  got:
 			ext4_group_desc_csum_set(sb, group, gdp);
 		}
 		ext4_unlock_group(sb, group);
+		brelse(block_bitmap_bh);
 
 		if (err)
 			goto fail;