public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
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;
>  	}
> 

  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