On Mon, Jan 13, 2025 at 11:33 AM Theodore Ts'o <tytso@xxxxxxx> wrote: > The fix might be as simple as this, but I haven't had a chance to test > it and do appropriate regression tests.... > > diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c > index eb73922d3..e460a75f4 100644 > --- a/e2fsck/pass1.c > +++ b/e2fsck/pass1.c > @@ -3842,7 +3842,7 @@ static int process_block(ext2_filsys fs, > problem = PR_1_TOOBIG_DIR; > if (p->is_dir && p->num_blocks + 1 >= p->max_blocks) > problem = PR_1_TOOBIG_DIR; > - if (p->is_reg && p->num_blocks + 1 >= p->max_blocks) > + if (p->is_reg && p->num_blocks + 1 >= 1U << 31) > problem = PR_1_TOOBIG_REG; > if (!p->is_dir && !p->is_reg && blockcnt > 0) > problem = PR_1_TOOBIG_SYMLINK; I can confirm that with this patch, e2fsck passes on the test image created as shown in my original email (dd if=/dev/zero ...). I also confirm 'make check' passes (390 tests succeeded). Do you have any thoughts on what a practical regression test would look like? My repro instructions require 2.1 TB of physical disk space and root access, which I am guessing is out of the question. For my local tests I have been using 'qemu-nbd' and QCOW2 images to reduce the disk space requirements, but it still requires root and ~30 minute runtime, which still seems impractical. > ind/dind/tind blocks, etc. We do care about num_blocks being too big > for the !huge_file case since for !huge_file file systems i_blocks is > denominated in 512 byte units, and is only 32-bits wide. So in that > case, we *do* care about the size of the file including metadata > blocks being no more than 2TiB. In the proposed patch, "p->num_blocks + 1 >= 1U << 31", that's 2^31 512-byte blocks, would that limit file size to 1 TB? > You're right though that we shouldn't be using num_blocks at all for > testing for regular files or directory files that are too big, since > num_blocks include blocks for extended attribute blocks, the > ind/dind/tind blocks, etc. We do care about num_blocks being too big > for the !huge_file case since for !huge_file file systems i_blocks is > denominated in 512 byte units, and is only 32-bits wide. So in that > case, we *do* care about the size of the file including metadata > blocks being no more than 2TiB. > > - Ted >
