[GIT PULL] e4defrag inline data segfault fix

[GIT PULL] e4defrag inline data segfault fix

[Daniel Tang]

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 .

Re: [GIT PULL] e4defrag inline data segfault fix

[Darrick J. Wong]

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?

--D

Re: [GIT PULL] e4defrag inline data segfault fix

[Theodore Tso]

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;
 	}

Re: [GIT PULL] e4defrag inline data segfault fix

[Darrick J. Wong]

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;
>  	}
> 

Re: [GIT PULL] e4defrag inline data segfault fix

[Theodore Tso]

On Fri, Jan 16, 2026 at 06:35:59PM -0800, Darrick J. Wong wrote:
> > -	/* 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?

I could have checked for EXT4_INLINE_DATA_FL, but we need to call
stat(2) to check for the st_blocks == 0 case, and while it is harmless
to defrag a file with a single data block, it's also pointless and a
waste of system calls.  So it's best that we skip defragging the file
in these cases:

  A) A zero-length file with st_blocks == 0
  B) A file with a single data block (st_blocks == st_blksize / 512)
  C) A file with inline data (st_blocks == 1)

... and we can do that only by checking the values returned by
stat(2).

Yes, (B) and (C) relies on Linux's behavior, since Posix is silent on
the semantics of st_blocks, but e4defrag works only by using a
Linux-specific ioctl, and using FS_IOC_GETFLAGS would also be
Linux-only.

    	       	       	       		- Ted

Re: [GIT PULL] e4defrag inline data segfault fix

[Darrick J. Wong]

On Fri, Jan 16, 2026 at 05:47:47PM -1000, Theodore Tso wrote:
> On Fri, Jan 16, 2026 at 06:35:59PM -0800, Darrick J. Wong wrote:
> > > -	/* 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?
> 
> I could have checked for EXT4_INLINE_DATA_FL, but we need to call
> stat(2) to check for the st_blocks == 0 case, and while it is harmless
> to defrag a file with a single data block, it's also pointless and a
> waste of system calls.  So it's best that we skip defragging the file
> in these cases:
> 
>   A) A zero-length file with st_blocks == 0
>   B) A file with a single data block (st_blocks == st_blksize / 512)
>   C) A file with inline data (st_blocks == 1)
> 
> ... and we can do that only by checking the values returned by
> stat(2).
> 
> Yes, (B) and (C) relies on Linux's behavior, since Posix is silent on
> the semantics of st_blocks, but e4defrag works only by using a
> Linux-specific ioctl, and using FS_IOC_GETFLAGS would also be
> Linux-only.

Fair enough.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


>     	       	       	       		- Ted
>