ext4: Fix possible use after free of buffer head

Submitted by Lukas Czerner on Nov. 29, 2012, 10:43 a.m.

Details

Message ID 1354185828-28545-1-git-send-email-lczerner@redhat.com
State Accepted, archived
Headers show

Commit Message

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

Comments

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 hide | download patch | download mbox

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;