linux-ext4 - Re: [GIT PULL] e4defrag inline data segfault fix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260117023559.GC15522@frogsfrogsfrogs>
Date: Fri, 16 Jan 2026 18:35:59 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Theodore Tso <tytso@....edu>
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 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@....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@....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;
>  	}
>