From: "Darrick J. Wong" <djwong@kernel.org>
To: Theodore Tso <tytso@mit.edu>
Cc: Daniel Tang <danielzgtg.opensource@gmail.com>,
linux-ext4@vger.kernel.org
Subject: Re: [GIT PULL] e4defrag inline data segfault fix
Date: Fri, 16 Jan 2026 18:35:59 -0800 [thread overview]
Message-ID: <20260117023559.GC15522@frogsfrogsfrogs> (raw)
In-Reply-To: <20260116234405.GG19200@macsyma.local>
On Fri, Jan 16, 2026 at 01:44:05PM -1000, Theodore Tso wrote:
> On Fri, Jan 16, 2026 at 09:21:39AM -0800, Darrick J. Wong wrote:
> > On Fri, Jan 16, 2026 at 05:25:35AM -0500, Daniel Tang wrote:
> > > Please sign-off on, and apply, the patch at
> > > https://gist.github.com/tytso/609572aed4d3f789742a50356567e929 . It
> > > fixes the bug at https://github.com/tytso/e2fsprogs/issues/260 .
> >
> > Perhaps that patch should get posted to the list for a proper review?
>
> The context was that Daniel had proposed a pull request on github:
>
> https://github.com/tytso/e2fsprogs/pull/261
>
> I had reviewed the change on github, and counter-proposed a better
> fix, which was what Daniel was referring to on the gist.github.com,
> and asked him to confirm that this fixed the issue that he was
> concerned about.
>
> This took place in early Ddecember, and I lost track of it because of
> the holidays. (Daniel, that's because my primary workflow is e-mail,
> and github issues and pull requests are things that I look at on a
> best-efforts basis, whereas with e-mail I have things like Patchwork
> to make sure I don't lose track of patch discussions. It also means
> that other people can more easily review proposed fixes.)
>
> Anyway, for folks on the ext4 list who might be curious, here's the
> fix. As it turns out, this is one where the description of the fix
> takes a lot more space than the actual fix itself. Which is why I
> hadn't bothered to write it all up before asking Daniel to test it to
> make sure it fixed the issue that he had run into.
<nod> bugfixes are fun like that :D
> - Ted
>
> commit 23785e90554b301b90076568fd33eb76dc930fba
> Author: Theodore Ts'o <tytso@mit.edu>
> Date: Fri Jan 16 18:01:09 2026 -0500
>
> e4defrag: don't try to defragment files which have 0 or 1 blocks
>
> This fixes a crash in e4defrag when the file is using the inline_data
> feature, and so the file data is stored in the inode table.
> Technically speaking, such a file does not consume any data blocks,
> but when an application program calls stat(2) on such a file, and
> st_blocks is set to 0, it might confuse the program into thinking the
> file did not contain any data. For this reason, ext4 returns 1 in
> st_blocks. (For other files or directories, st_blocks will be a
> multiple of the file system block size divided by 512, since st_blocks
> is denominated in units of 512 sectors on Linux --- and most other Unix
> systems with the notable exception of HP-UX.)
>
> Unfortunately, when e4defrag tries to defragment a inline data file,
> it divides st_blocks by (fs->block_size / 512), and this results in
> e4defrag thinking that the file 0 data blocks --- and since the file
> is not skipped because st_blocks != 0, this results in crash when
> dividing by zero.
>
> As it turns out, it's pointless to try to defrag a file with 0 or 1
> data blocks. So fix this by skipping any file where st_blocks <=
> block_size / 512.
Excellent description. I glanced at the github issue this morning and
wondered what was going on with all the /512s...
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>
> diff --git a/misc/e4defrag.c b/misc/e4defrag.c
> index bbeb5b167..68e937fdb 100644
> --- a/misc/e4defrag.c
> +++ b/misc/e4defrag.c
> @@ -1091,11 +1091,11 @@ static int file_statistic(const char *file, const struct stat64 *buf,
> return 0;
> }
>
> - /* Has no blocks */
> - if (buf->st_blocks == 0) {
> + /* Has 0 or 1 blocks, no point to defragment */
> + if (buf->st_blocks <= buf->st_blksize / 512) {
...because can't you call FS_IOC_GETFLAGS and look for
EXT4_INLINE_DATA_FL?
--D
> if (mode_flag & DETAIL) {
> PRINT_FILE_NAME(file);
> - STATISTIC_ERR_MSG("File has no blocks");
> + STATISTIC_ERR_MSG("# of file blocks <= 1");
> }
> return 0;
> }
> @@ -1452,11 +1452,11 @@ static int file_defrag(const char *file, const struct stat64 *buf,
> return 0;
> }
>
> - /* Has no blocks */
> - if (buf->st_blocks == 0) {
> + /* Has 0 or 1 blocks, no point to defragment */
> + if (buf->st_blocks <= buf->st_blksize / 512) {
> if (mode_flag & DETAIL) {
> PRINT_FILE_NAME(file);
> - STATISTIC_ERR_MSG("File has no blocks");
> + IN_FTW_PRINT_ERR_MSG("# of file blocks <= 1");
> }
> return 0;
> }
>
next prev parent reply other threads:[~2026-01-17 2:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-16 10:25 [GIT PULL] e4defrag inline data segfault fix Daniel Tang
2026-01-16 17:21 ` Darrick J. Wong
2026-01-16 23:44 ` Theodore Tso
2026-01-17 2:35 ` Darrick J. Wong [this message]
2026-01-17 3:47 ` Theodore Tso
2026-01-17 4:49 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260117023559.GC15522@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=danielzgtg.opensource@gmail.com \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox