public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Tso" <tytso@mit.edu>
To: "Darrick J. Wong" <djwong@kernel.org>
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 13:44:05 -1000	[thread overview]
Message-ID: <20260116234405.GG19200@macsyma.local> (raw)
In-Reply-To: <20260116172139.GB15522@frogsfrogsfrogs>

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@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.
    
    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) {
 		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;
 	}

  reply	other threads:[~2026-01-16 23:44 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 [this message]
2026-01-17  2:35     ` Darrick J. Wong
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=20260116234405.GG19200@macsyma.local \
    --to=tytso@mit.edu \
    --cc=danielzgtg.opensource@gmail.com \
    --cc=djwong@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    /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