[PATCH] ext4: verify dirent offset in ext4_readdir()

[PATCH] ext4: verify dirent offset in ext4_readdir()

[Dmitry Antipov]

On a corrupted filesystem, an unexpectedly large invalid value
returned by 'ext4_rec_len_from_disk()' may cause 'ext4_readdir()'
to read the next dirent from an area beyond the corresponding
buffer head's data. At this point, an exact length of the dirent
is not known yet but it's possible to check whether the shortest
possible dirent will be read from within the bh's data at least.

Reported-by: syzbot+5322c5c260eb44d209ed@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5322c5c260eb44d209ed
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 fs/ext4/dir.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index d4164c507a90..8097016f69aa 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -258,6 +258,12 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
 
 		while (ctx->pos < inode->i_size
 		       && offset < sb->s_blocksize) {
+			/* Ensure that at least the shortest possible
+			 * dirent will be read from within the bh's data.
+			 */
+			if (offset + offsetof(struct ext4_dir_entry_2, name)
+			    > bh->b_size)
+				break;
 			de = (struct ext4_dir_entry_2 *) (bh->b_data + offset);
 			if (ext4_check_dir_entry(inode, file, de, bh,
 						 bh->b_data, bh->b_size,
-- 
2.50.0

Re: [PATCH] ext4: verify dirent offset in ext4_readdir()

[Darrick J. Wong]

On Tue, Jul 01, 2025 at 05:11:41PM +0300, Dmitry Antipov wrote:
> On a corrupted filesystem, an unexpectedly large invalid value
> returned by 'ext4_rec_len_from_disk()' may cause 'ext4_readdir()'
> to read the next dirent from an area beyond the corresponding
> buffer head's data. At this point, an exact length of the dirent
> is not known yet but it's possible to check whether the shortest
> possible dirent will be read from within the bh's data at least.
> 
> Reported-by: syzbot+5322c5c260eb44d209ed@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5322c5c260eb44d209ed
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  fs/ext4/dir.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index d4164c507a90..8097016f69aa 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -258,6 +258,12 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
>  
>  		while (ctx->pos < inode->i_size
>  		       && offset < sb->s_blocksize) {
> +			/* Ensure that at least the shortest possible
> +			 * dirent will be read from within the bh's data.
> +			 */
> +			if (offset + offsetof(struct ext4_dir_entry_2, name)
> +			    > bh->b_size)
> +				break;

Why wouldn't you encode this check in __ext4_check_dir_entry and solve
this problem for all the callsites?

--D

>  			de = (struct ext4_dir_entry_2 *) (bh->b_data + offset);
>  			if (ext4_check_dir_entry(inode, file, de, bh,
>  						 bh->b_data, bh->b_size,
> -- 
> 2.50.0
> 
> 

Re: [PATCH] ext4: verify dirent offset in ext4_readdir()

[Dmitry Antipov]

On 7/2/25 6:23 PM, Darrick J. Wong wrote:

> Why wouldn't you encode this check in __ext4_check_dir_entry and solve
> this problem for all the callsites?

Next thing to try indeed.

BTW, looking through ext4_search_dir(), why the search doesn't
actually start from the specified offset? I.e. shouldn't it be:

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a178ac229489..8aa0d68dae71 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1461,7 +1461,7 @@ int ext4_search_dir(struct buffer_head *bh, char *search_buf, int buf_size,
         char * dlimit;
         int de_len;

-       de = (struct ext4_dir_entry_2 *)search_buf;
+       de = (struct ext4_dir_entry_2 *)search_buf + offset;
         dlimit = search_buf + buf_size;
         while ((char *) de < dlimit - EXT4_BASE_DIR_LEN) {
                 /* this code is executed quadratically often */

Dmitry

Re: [PATCH] ext4: verify dirent offset in ext4_readdir()

[Theodore Ts'o]

On Thu, Jul 03, 2025 at 12:54:06PM +0300, Dmitry Antipov wrote:
> On 7/2/25 6:23 PM, Darrick J. Wong wrote:
> 
> > Why wouldn't you encode this check in __ext4_check_dir_entry and solve
> > this problem for all the callsites?
> 
> Next thing to try indeed.
> 
> BTW, looking through ext4_search_dir(), why the search doesn't
> actually start from the specified offset? I.e. shouldn't it be:

ext4_search_dir() always searches the entire directory block.  The
offset is relative to the beginning of the directory, and it's used
only printing error messages so someone who is debugging a file system
failure knows where in the directory the corruption was found:

		ext4_error_file(filp, function, line, bh->b_blocknr,
				"bad entry in directory: %s - offset=%u, "
				"inode=%u, rec_len=%d, size=%d fake=%d",
				error_msg, offset, le32_to_cpu(de->inode),
				rlen, size, fake);

Offset can (and very often will be) significanty larger tan the size
of search_buf, so 

> +       de = (struct ext4_dir_entry_2 *)search_buf + offset;

would be practcally guaranteed to induce an out-of-bounds read.

      	 	    	       	  	    		  - Ted

Re: [PATCH] ext4: verify dirent offset in ext4_readdir()

[Theodore Ts'o]

On Wed, Jul 02, 2025 at 08:23:04AM -0700, Darrick J. Wong wrote:
> 
> Why wouldn't you encode this check in __ext4_check_dir_entry and solve
> this problem for all the callsites?

More to the point, why wasn't this caught when checking the previous
directory entry in __ext4_check_ir_entry() via this:

	else if (unlikely(next_offset > size - ext4_dir_rec_len(1,
						  has_csum ? NULL : dir) &&
			  next_offset != size))
		error_msg = "directory entry too close to block end";

This patch claims to address a syzbot report, but it currently
doesn't have a C reprducer.  Are we sure that this change actually
makes a difference?

					- Ted