Theodore Ts'o (7):
e2fsck: sanity check the journal inode number
e2fsck: fix potential out-of-bounds read in inc_ea_inode_refs()
libext2fs: add check for too-short directory blocks
e2fsck: check for xattr value size integer wraparound
e2fsck: avoid out-of-bounds write for very deep extent trees
libext2fs: check for cyclic loops in the extent tree
libext2fs: check for invalid blocks in ext2fs_punch_blocks()
e2fsck/extents.c | 10 +++++++++-
e2fsck/journal.c | 9 ++++++++-
e2fsck/pass1.c | 21 +++++++++++++--------
lib/ext2fs/alloc_stats.c | 3 ++-
lib/ext2fs/dir_iterate.c | 4 ++++
lib/ext2fs/ext2_err.et.in | 3 +++
lib/ext2fs/ext2_ext_attr.h | 11 +++++++++++
lib/ext2fs/extent.c | 11 +++++++++--
lib/ext2fs/punch.c | 4 ++++
9 files changed, 63 insertions(+), 13 deletions(-)
--
2.31.0
If there is an inline data directory which is smaller than 8 bytes
(which should never happen but for corrupted or fuzzed file systems),
ext2fs_process_dir_block() will now abort EXT2_ET_DIR_CORRUPTED to
avoid an out-of-bounds read.
Reported-by: Nils Bars <[email protected]>
Reported-by: Moritz Schlögel <[email protected]>
Reported-by: Nico Schiller <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
---
lib/ext2fs/dir_iterate.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/ext2fs/dir_iterate.c b/lib/ext2fs/dir_iterate.c
index b2b77693..7798a482 100644
--- a/lib/ext2fs/dir_iterate.c
+++ b/lib/ext2fs/dir_iterate.c
@@ -221,6 +221,10 @@ int ext2fs_process_dir_block(ext2_filsys fs,
if (ext2fs_has_feature_metadata_csum(fs->super))
csum_size = sizeof(struct ext2_dir_entry_tail);
+ if (buflen < 8) {
+ ctx->errcode = EXT2_ET_DIR_CORRUPTED;
+ return BLOCK_ABORT;
+ }
while (offset < buflen - 8) {
dirent = (struct ext2_dir_entry *) (ctx->buf + offset);
if (ext2fs_get_rec_len(fs, dirent, &rec_len))
--
2.31.0
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 <[email protected]>
Reported-by: Moritz Schlögel <[email protected]>
Reported-by: Nico Schiller <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
---
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
On Tue, Jun 07, 2022 at 12:24:40AM -0400, Theodore Ts'o wrote:
> If there is an inline data directory which is smaller than 8 bytes
> (which should never happen but for corrupted or fuzzed file systems),
> ext2fs_process_dir_block() will now abort EXT2_ET_DIR_CORRUPTED to
> avoid an out-of-bounds read.
Looks good.
Reviewed-by: Lukas Czerner <[email protected]>
>
> Reported-by: Nils Bars <[email protected]>
> Reported-by: Moritz Schl?gel <[email protected]>
> Reported-by: Nico Schiller <[email protected]>
> Signed-off-by: Theodore Ts'o <[email protected]>
> ---
> lib/ext2fs/dir_iterate.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/lib/ext2fs/dir_iterate.c b/lib/ext2fs/dir_iterate.c
> index b2b77693..7798a482 100644
> --- a/lib/ext2fs/dir_iterate.c
> +++ b/lib/ext2fs/dir_iterate.c
> @@ -221,6 +221,10 @@ int ext2fs_process_dir_block(ext2_filsys fs,
> if (ext2fs_has_feature_metadata_csum(fs->super))
> csum_size = sizeof(struct ext2_dir_entry_tail);
>
> + if (buflen < 8) {
> + ctx->errcode = EXT2_ET_DIR_CORRUPTED;
> + return BLOCK_ABORT;
> + }
> while (offset < buflen - 8) {
> dirent = (struct ext2_dir_entry *) (ctx->buf + offset);
> if (ext2fs_get_rec_len(fs, dirent, &rec_len))
> --
> 2.31.0
>
If there isn't enough space for a full extended attribute entry,
inc_ea_inode_refs() might end up reading beyond the allocated memory
buffer.
Reported-by: Nils Bars <[email protected]>
Reported-by: Moritz Schlögel <[email protected]>
Reported-by: Nico Schiller <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
---
e2fsck/pass1.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index dde862a8..2a17bb8a 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -389,13 +389,13 @@ static problem_t check_large_ea_inode(e2fsck_t ctx,
static void inc_ea_inode_refs(e2fsck_t ctx, struct problem_context *pctx,
struct ext2_ext_attr_entry *first, void *end)
{
- struct ext2_ext_attr_entry *entry;
+ struct ext2_ext_attr_entry *entry = first;
+ struct ext2_ext_attr_entry *np = EXT2_EXT_ATTR_NEXT(entry);
- for (entry = first;
- (void *)entry < end && !EXT2_EXT_IS_LAST_ENTRY(entry);
- entry = EXT2_EXT_ATTR_NEXT(entry)) {
+ while ((void *) entry < end && (void *) np < end &&
+ !EXT2_EXT_IS_LAST_ENTRY(entry)) {
if (!entry->e_value_inum)
- continue;
+ goto next;
if (!ctx->ea_inode_refs) {
pctx->errcode = ea_refcount_create(0,
&ctx->ea_inode_refs);
@@ -408,6 +408,9 @@ static void inc_ea_inode_refs(e2fsck_t ctx, struct problem_context *pctx,
}
ea_refcount_increment(ctx->ea_inode_refs, entry->e_value_inum,
0);
+ next:
+ entry = np;
+ np = EXT2_EXT_ATTR_NEXT(entry);
}
}
--
2.31.0
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 <[email protected]>
>
> Reported-by: Nils Bars <[email protected]>
> Reported-by: Moritz Schl?gel <[email protected]>
> Reported-by: Nico Schiller <[email protected]>
> Signed-off-by: Theodore Ts'o <[email protected]>
> ---
> 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
>