diff mbox series

[5/7] e2fsck: avoid out-of-bounds write for very deep extent trees

Message ID 20220607042444.1798015-6-tytso@mit.edu
State Accepted
Headers show
Series Fix various bugs found via a fuzzing campaign | expand

Commit Message

Theodore Ts'o June 7, 2022, 4:24 a.m. UTC
The kernel doesn't support extent trees deeper than 5
(EXT4_MAX_EXTENT_DEPTH).  For this reason we only maintain the extent
tree statistics for 5 levels.  Avoid out-of-bounds writes and reads if
the extent tree is deeper than this.

We keep these statistics to determine whether we should rebuild the
extent tree.  If the extent tree is too deep, we don't need the
statistics because we should always rebuild the it.

Reported-by: Nils Bars <nils.bars@rub.de>
Reported-by: Moritz Schlögel <moritz.schloegel@rub.de>
Reported-by: Nico Schiller <nico.schiller@rub.de>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 e2fsck/extents.c | 10 +++++++++-
 e2fsck/pass1.c   |  3 ++-
 2 files changed, 11 insertions(+), 2 deletions(-)

Comments

Lukas Czerner June 7, 2022, 1:53 p.m. UTC | #1
On Tue, Jun 07, 2022 at 12:24:42AM -0400, Theodore Ts'o wrote:
> The kernel doesn't support extent trees deeper than 5
> (EXT4_MAX_EXTENT_DEPTH).  For this reason we only maintain the extent
> tree statistics for 5 levels.  Avoid out-of-bounds writes and reads if
> the extent tree is deeper than this.
> 
> We keep these statistics to determine whether we should rebuild the
> extent tree.  If the extent tree is too deep, we don't need the
> statistics because we should always rebuild the it.

Looks good.

Reviewed-by: Lukas Czerner <lczerner@redhat.com>

> 
> Reported-by: Nils Bars <nils.bars@rub.de>
> Reported-by: Moritz Schlögel <moritz.schloegel@rub.de>
> Reported-by: Nico Schiller <nico.schiller@rub.de>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  e2fsck/extents.c | 10 +++++++++-
>  e2fsck/pass1.c   |  3 ++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/e2fsck/extents.c b/e2fsck/extents.c
> index 01879f56..86fe00e7 100644
> --- a/e2fsck/extents.c
> +++ b/e2fsck/extents.c
> @@ -526,7 +526,8 @@ errcode_t e2fsck_check_rebuild_extents(e2fsck_t ctx, ext2_ino_t ino,
>  		 */
>  		if (info.curr_entry == 1 &&
>  		    !(extent.e_flags & EXT2_EXTENT_FLAGS_SECOND_VISIT) &&
> -		    !eti.force_rebuild) {
> +		    !eti.force_rebuild &&
> +		    info.curr_level < MAX_EXTENT_DEPTH_COUNT) {
>  			struct extent_tree_level *etl;
>  
>  			etl = eti.ext_info + info.curr_level;
> @@ -580,6 +581,13 @@ errcode_t e2fsck_should_rebuild_extents(e2fsck_t ctx,
>  	extents_per_block = (ctx->fs->blocksize -
>  			     sizeof(struct ext3_extent_header)) /
>  			    sizeof(struct ext3_extent);
> +
> +	/* If the extent tree is too deep, then rebuild it. */
> +	if (info->max_depth > MAX_EXTENT_DEPTH_COUNT) {
> +		pctx->blk = info->max_depth;
> +		op = PR_1E_CAN_COLLAPSE_EXTENT_TREE;
> +		goto rebuild;
> +	}
>  	/*
>  	 * If we can consolidate a level or shorten the tree, schedule the
>  	 * extent tree to be rebuilt.
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 11d7ce93..43972e7c 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -2842,7 +2842,8 @@ static void scan_extent_node(e2fsck_t ctx, struct problem_context *pctx,
>  	if (pctx->errcode)
>  		return;
>  	if (!(ctx->options & E2F_OPT_FIXES_ONLY) &&
> -	    !pb->eti.force_rebuild) {
> +	    !pb->eti.force_rebuild &&
> +	    info.curr_level < MAX_EXTENT_DEPTH_COUNT) {
>  		struct extent_tree_level *etl;
>  
>  		etl = pb->eti.ext_info + info.curr_level;
> -- 
> 2.31.0
>
diff mbox series

Patch

diff --git a/e2fsck/extents.c b/e2fsck/extents.c
index 01879f56..86fe00e7 100644
--- a/e2fsck/extents.c
+++ b/e2fsck/extents.c
@@ -526,7 +526,8 @@  errcode_t e2fsck_check_rebuild_extents(e2fsck_t ctx, ext2_ino_t ino,
 		 */
 		if (info.curr_entry == 1 &&
 		    !(extent.e_flags & EXT2_EXTENT_FLAGS_SECOND_VISIT) &&
-		    !eti.force_rebuild) {
+		    !eti.force_rebuild &&
+		    info.curr_level < MAX_EXTENT_DEPTH_COUNT) {
 			struct extent_tree_level *etl;
 
 			etl = eti.ext_info + info.curr_level;
@@ -580,6 +581,13 @@  errcode_t e2fsck_should_rebuild_extents(e2fsck_t ctx,
 	extents_per_block = (ctx->fs->blocksize -
 			     sizeof(struct ext3_extent_header)) /
 			    sizeof(struct ext3_extent);
+
+	/* If the extent tree is too deep, then rebuild it. */
+	if (info->max_depth > MAX_EXTENT_DEPTH_COUNT) {
+		pctx->blk = info->max_depth;
+		op = PR_1E_CAN_COLLAPSE_EXTENT_TREE;
+		goto rebuild;
+	}
 	/*
 	 * If we can consolidate a level or shorten the tree, schedule the
 	 * extent tree to be rebuilt.
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 11d7ce93..43972e7c 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -2842,7 +2842,8 @@  static void scan_extent_node(e2fsck_t ctx, struct problem_context *pctx,
 	if (pctx->errcode)
 		return;
 	if (!(ctx->options & E2F_OPT_FIXES_ONLY) &&
-	    !pb->eti.force_rebuild) {
+	    !pb->eti.force_rebuild &&
+	    info.curr_level < MAX_EXTENT_DEPTH_COUNT) {
 		struct extent_tree_level *etl;
 
 		etl = pb->eti.ext_info + info.curr_level;