[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260116234405.GG19200@macsyma.local>
Date: Fri, 16 Jan 2026 13:44:05 -1000
From: "Theodore Tso" <tytso@....edu>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: Daniel Tang <danielzgtg.opensource@...il.com>, linux-ext4@...r.kernel.org
Subject: Re: [GIT PULL] e4defrag inline data segfault fix
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.
- Ted
commit 23785e90554b301b90076568fd33eb76dc930fba
Author: Theodore Ts'o <tytso@....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.
Signed-off-by: Theodore Ts'o <tytso@....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) {
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;
}
Powered by blists - more mailing lists