linux-ext4 - Re: e2fsck max blocks for huge non-extent file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAE2LqHJ0y1OBDKv84PuVVi2JULvuVA8EWJpXbN9vdK9o7LEOhw@mail.gmail.com>
Date: Thu, 16 Jan 2025 22:26:36 -0500
From: Catalin Patulea <cronos586@...il.com>
To: "Theodore Ts'o" <tytso@....edu>
Cc: "Darrick J. Wong" <djwong@...nel.org>, linux-ext4@...r.kernel.org, 
	Kazuya Mio <k-mio@...jp.nec.com>
Subject: Re: e2fsck max blocks for huge non-extent file

On Mon, Jan 13, 2025 at 11:33 AM Theodore Ts'o <tytso@....edu> 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
>