All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
@ 2026-08-13  8:33 pipishuo
  2026-08-13  8:49 ` sashiko-bot
  2026-08-14  3:31 ` Theodore Tso
  0 siblings, 2 replies; 10+ messages in thread
From: pipishuo @ 2026-08-13  8:33 UTC (permalink / raw)
  To: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
	yi.zhang
  Cc: linux-ext4

After a failure in the route ext4_restore_inline_data ->
ext4_create_inline_data -> ext4_mark_iloc_dirty, the inode will still
be considered as having inline data. However, in ext4_restore_inline_data,
the inline data is not actually written to the inode. When ext4_readdir
is subsequently called,ext4_has_inline_data will pass the check
and proceed to read the incorrect inline data.

Signed-off-by: pipishuo <1289151713@qq.com>
---
 fs/ext4/dir.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 17edd678fa87..6117a798541a 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -22,6 +22,7 @@
  *
  */
 
+#include <asm-generic/errno-base.h>
 #include <linux/fs.h>
 #include <linux/buffer_head.h>
 #include <linux/filelock.h>
@@ -159,6 +160,9 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
 	}
 
 	if (ext4_has_inline_data(inode)) {
+		if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
+			return -EINVAL;
+
 		int has_inline_data = 1;
 		err = ext4_read_inline_dir(file, ctx,
 					   &has_inline_data);
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-13  8:33 [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir pipishuo
@ 2026-08-13  8:49 ` sashiko-bot
  2026-08-14  1:27   ` shuo chen
  2026-08-14  3:31 ` Theodore Tso
  1 sibling, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-08-13  8:49 UTC (permalink / raw)
  To: pipishuo; +Cc: tytso, linux-ext4

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning `-EINVAL` from `ext4_readdir` on a corrupted or inconsistent directory state breaks userspace expectations and bypasses filesystem error handling.
--

--- Patch [1]: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir ---
commit 419471105d79565b3d6563ce7e941b266526160b
Author: pipishuo <1289151713@qq.com>

Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

This commit adds a check for EXT4_STATE_MAY_INLINE_DATA in ext4_readdir to
prevent reading incorrect inline data if the inode's inline state is
inconsistent after a failure in ext4_restore_inline_data.

> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 8d7b81e6948e7..857cf02742c0c 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c

[ ... ]

> @@ -160,6 +161,9 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
>  	}
>  
>  	if (ext4_has_inline_data(inode)) {
> +		if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
> +			return -EINVAL;
> +

[Severity: High]
Does returning -EINVAL here break userspace expectations and bypass the
standard filesystem corruption handling?

If an operation (like ext4_convert_inline_data_nolock) fails and leaves
EXT4_INODE_INLINE_DATA set without EXT4_STATE_MAY_INLINE_DATA, this
indicates an internally corrupted or inconsistent directory state.

By returning -EINVAL directly, this code bypasses the standard
ext4_error_file() error handler, which prevents the filesystem from taking
corrective action (such as remounting read-only).

Additionally, returning -EINVAL from a readdir (getdents64) operation is
typically interpreted by userspace as a buffer size issue, which completely
misleads tools like ls. Should this instead invoke EXT4_ERROR_FILE() and
return -EFSCORRUPTED?

>  		int has_inline_data = 1;
>  		err = ext4_read_inline_dir(file, ctx,
>  					   &has_inline_data);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_321031C6B9D5D092C8978B46D9F0BBA18607@qq.com?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-13  8:49 ` sashiko-bot
@ 2026-08-14  1:27   ` shuo chen
  0 siblings, 0 replies; 10+ messages in thread
From: shuo chen @ 2026-08-14  1:27 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: tytso, linux-ext4

On Thu, Aug 13, 2026 at 08:49:27AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Returning `-EINVAL` from `ext4_readdir` on a corrupted or inconsistent directory state breaks userspace expectations and bypasses filesystem error handling.
> --
> 
> --- Patch [1]: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir ---
> commit 419471105d79565b3d6563ce7e941b266526160b
> Author: pipishuo <1289151713@qq.com>
> 
> Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
> 
> This commit adds a check for EXT4_STATE_MAY_INLINE_DATA in ext4_readdir to
> prevent reading incorrect inline data if the inode's inline state is
> inconsistent after a failure in ext4_restore_inline_data.
> 
> > diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> > index 8d7b81e6948e7..857cf02742c0c 100644
> > --- a/fs/ext4/dir.c
> > +++ b/fs/ext4/dir.c
> 
> [ ... ]
> 
> > @@ -160,6 +161,9 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
> >  	}
> >  
> >  	if (ext4_has_inline_data(inode)) {
> > +		if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
> > +			return -EINVAL;
> > +
> 
> [Severity: High]
> Does returning -EINVAL here break userspace expectations and bypass the
> standard filesystem corruption handling?
> 
> If an operation (like ext4_convert_inline_data_nolock) fails and leaves
> EXT4_INODE_INLINE_DATA set without EXT4_STATE_MAY_INLINE_DATA, this
> indicates an internally corrupted or inconsistent directory state.
> 
> By returning -EINVAL directly, this code bypasses the standard
> ext4_error_file() error handler, which prevents the filesystem from taking
> corrective action (such as remounting read-only).
> 
> Additionally, returning -EINVAL from a readdir (getdents64) operation is
> typically interpreted by userspace as a buffer size issue, which completely
> misleads tools like ls. Should this instead invoke EXT4_ERROR_FILE() and
> return -EFSCORRUPTED?
Thanks for the reviews. I'll address all the comments and send v2 soon.
> 
> >  		int has_inline_data = 1;
> >  		err = ext4_read_inline_dir(file, ctx,
> >  					   &has_inline_data);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/tencent_321031C6B9D5D092C8978B46D9F0BBA18607@qq.com?part=1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-13  8:33 [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir pipishuo
  2026-08-13  8:49 ` sashiko-bot
@ 2026-08-14  3:31 ` Theodore Tso
  2026-08-14  8:40   ` shuo chen
  2026-08-16 15:02   ` [PATCH v2] " shuo chen
  1 sibling, 2 replies; 10+ messages in thread
From: Theodore Tso @ 2026-08-14  3:31 UTC (permalink / raw)
  To: pipishuo
  Cc: adilger.kernel, libaokun, jack, ojaswin, ritesh.list, yi.zhang,
	linux-ext4

On Thu, Aug 13, 2026 at 04:33:57PM -0500, pipishuo wrote:
> After a failure in the route ext4_restore_inline_data ->
> ext4_create_inline_data -> ext4_mark_iloc_dirty, the inode will still
> be considered as having inline data.

How did you run across this?  Was this something that was actually
happening?  If so, what was triggering the error?

Or was this something that was found via AI?

I'm fairly sure this patch was created using AI.  This useless
inclusion was a bit of a tip-off:

> +#include <asm-generic/errno-base.h>

As far as the fix is concerned, there's a much better way of
implementing the functionality in ext4_restore_inline_data(), which is
to just capture a copy of the inode (since the inline data is stored
in the inode), and if we need to back out a failed conversion in
ext4_convert_inline_data_nolock(), we can just copy the old contents
of the inode back in place.

We can also do the conversion in a much safer way, by allocating the
data block first, and writing a copy of the data to the data block,
and only if that is successful, do we update the extent tree or
indirect block mapping in the inode.  Of course, this would require a
rewrite of ext4_convert_inline_data_nolock(), but this would allow us
to fix up the other aspects of the error handling which is not quite
right.

						- Ted

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-14  3:31 ` Theodore Tso
@ 2026-08-14  8:40   ` shuo chen
  2026-08-14 13:57     ` Theodore Tso
  2026-08-16 15:02   ` [PATCH v2] " shuo chen
  1 sibling, 1 reply; 10+ messages in thread
From: shuo chen @ 2026-08-14  8:40 UTC (permalink / raw)
  To: Theodore Tso
  Cc: adilger.kernel, libaokun, jack, ojaswin, ritesh.list, yi.zhang,
	linux-ext4

On Thu, Aug 13, 2026 at 11:31:03PM -0400, Theodore Tso wrote:
> On Thu, Aug 13, 2026 at 04:33:57PM -0500, pipishuo wrote:
> > After a failure in the route ext4_restore_inline_data ->
> > ext4_create_inline_data -> ext4_mark_iloc_dirty, the inode will still
> > be considered as having inline data.
> 
> How did you run across this?  Was this something that was actually
> happening?  If so, what was triggering the error?
> 
> Or was this something that was found via AI?
I didn't have a reproducer. I was reviewing the inline data error paths 
and noticed the interaction with ext4_readdir looked fragile
so I attempted to make it more robust.
> 
> I'm fairly sure this patch was created using AI.  This useless
> inclusion was a bit of a tip-off:
> 
> > +#include <asm-generic/errno-base.h>
It was automatically inserted by my editor's include suggestion feature,
and I missed it in review. That's entirely my fault.
> 
> As far as the fix is concerned, there's a much better way of
> implementing the functionality in ext4_restore_inline_data(), which is
> to just capture a copy of the inode (since the inline data is stored
> in the inode), and if we need to back out a failed conversion in
> ext4_convert_inline_data_nolock(), we can just copy the old contents
> of the inode back in place.
Because ext4_convert_inline_data_nolock() has already called
ext4_destroy_inline_data_nolock(), the inode is no longer in the inline
state. So we need ext4_create_inline_data() to rebuild the inline state.
> 
> We can also do the conversion in a much safer way, by allocating the
> data block first, and writing a copy of the data to the data block,
> and only if that is successful, do we update the extent tree or
> indirect block mapping in the inode.  Of course, this would require a
> rewrite of ext4_convert_inline_data_nolock(), but this would allow us
> to fix up the other aspects of the error handling which is not quite
> right. 
I agree with your direction. I'll take some time to study the code 
and work on a proper v2 that rewrites ext4_convert_inline_data_nolock()
following that model.
> 
> 						- Ted


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-14  8:40   ` shuo chen
@ 2026-08-14 13:57     ` Theodore Tso
  2026-08-15  1:43       ` shuo chen
  0 siblings, 1 reply; 10+ messages in thread
From: Theodore Tso @ 2026-08-14 13:57 UTC (permalink / raw)
  To: shuo chen
  Cc: adilger.kernel, libaokun, jack, ojaswin, ritesh.list, yi.zhang,
	linux-ext4

On Fri, Aug 14, 2026 at 04:40:02PM -0500, shuo chen wrote:
> Because ext4_convert_inline_data_nolock() has already called
> ext4_destroy_inline_data_nolock(), the inode is no longer in the inline
> state. So we need ext4_create_inline_data() to rebuild the inline state.

There are other ways to restore the inline state than calling ext4_create_inline_data().

Cheers,

					- Ted

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-14 13:57     ` Theodore Tso
@ 2026-08-15  1:43       ` shuo chen
  0 siblings, 0 replies; 10+ messages in thread
From: shuo chen @ 2026-08-15  1:43 UTC (permalink / raw)
  To: Theodore Tso
  Cc: adilger.kernel, libaokun, jack, ojaswin, ritesh.list, yi.zhang,
	linux-ext4

On Fri, Aug 14, 2026 at 09:57:33AM -0400, Theodore Tso wrote:
> On Fri, Aug 14, 2026 at 04:40:02PM -0500, shuo chen wrote:
> > Because ext4_convert_inline_data_nolock() has already called
> > ext4_destroy_inline_data_nolock(), the inode is no longer in the inline
> > state. So we need ext4_create_inline_data() to rebuild the inline state.
> 
> There are other ways to restore the inline state than calling ext4_create_inline_data().
I get it — keep a backup of the inode and restore on rollback.
But I prefer your second (safer) approach — allocate first, then convert.
I'll send a v2 following that.
> 
> Cheers,
> 
> 					- Ted


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-14  3:31 ` Theodore Tso
  2026-08-14  8:40   ` shuo chen
@ 2026-08-16 15:02   ` shuo chen
  2026-08-16 15:16     ` sashiko-bot
  2026-08-16 20:05     ` [syzbot ci] " syzbot ci
  1 sibling, 2 replies; 10+ messages in thread
From: shuo chen @ 2026-08-16 15:02 UTC (permalink / raw)
  To: tytso
  Cc: adilger.kernel, libaokun, ack, ojaswin, yi.zhang, linux-ext4,
	shuo chen

> We can also do the conversion in a much safer way, by allocating the
> data block first, and writing a copy of the data to the data block,
> and only if that is successful, do we update the extent tree or
> indirect block mapping in the inode.  Of course, this would require a
> rewrite of ext4_convert_inline_data_nolock(), but this would allow us
> to fix up the other aspects of the error handling which is not quite
> right.
Rewrite ext4_convert_inline_data_nolock() as suggested.

---
v1 -> v2:
- Rewrite ext4_convert_inline_data_nolock() to allocate blocks before
  destroying inline data
- Link to v1: https://lore.kernel.org/linux-ext4/tencent_7A0B62235E38C5907A080BE3D56A59C9FD06@qq.com/T/#t:
---
Signed-off-by: shuo chen <1289151713@qq.com>
---
 fs/ext4/inline.c | 113 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 80 insertions(+), 33 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 8045e4ff270c..2a9299ea4146 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -14,6 +14,7 @@
 #include "ext4.h"
 #include "xattr.h"
 #include "truncate.h"
+#include "ext4_extents.h"
 
 #define EXT4_XATTR_SYSTEM_DATA	"data"
 #define EXT4_MIN_INLINE_DATA_SIZE	((sizeof(__le32) * EXT4_N_BLOCKS))
@@ -1070,21 +1071,67 @@ static int ext4_update_inline_dir(handle_t *handle, struct inode *dir,
 	return 0;
 }
 
-static void ext4_restore_inline_data(handle_t *handle, struct inode *inode,
-				     struct ext4_iloc *iloc,
-				     void *buf, int inline_size)
+static int ext4_set_inline_data_block(handle_t *handle, struct inode *inode,
+				      ext4_fsblk_t block, unsigned int len)
 {
-	int ret;
+	struct ext4_inode_info *ei = EXT4_I(inode);
+	struct ext4_xattr_ibody_find is = {
+		.s = { .not_found = 0, },
+	};
+	struct ext4_xattr_info i = {
+		.name_index = EXT4_XATTR_INDEX_SYSTEM,
+		.name = EXT4_XATTR_SYSTEM_DATA,
+		.value = NULL,
+		.value_len = 0,
+	};
+	int error;
 
-	ret = ext4_create_inline_data(handle, inode, inline_size);
-	if (ret) {
-		ext4_msg(inode->i_sb, KERN_EMERG,
-			"error restoring inline_data for inode -- potential data loss! (inode %llu, error %d)",
-			inode->i_ino, ret);
-		return;
+	down_write(&ei->i_data_sem);
+	error = ext4_get_inode_loc(inode, &is.iloc);
+	if (error) {
+		up_write(&ei->i_data_sem);
+		return error;
 	}
-	ext4_write_inline_data(inode, iloc, buf, 0, inline_size);
-	ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
+	error = ext4_xattr_ibody_find(inode, &i, &is);
+	if (error)
+		goto out;
+	BUFFER_TRACE(is.iloc.bh, "get_write_access");
+	error = ext4_journal_get_write_access(handle, inode->i_sb, is.iloc.bh, EXT4_JTR_NONE);
+	if (error)
+		goto out;
+	error = ext4_xattr_ibody_set(handle, inode, &i, &is);
+	if (error)
+		goto out;
+	memset((void *)ext4_raw_inode(&is.iloc)->i_block,
+		0, EXT4_MIN_INLINE_DATA_SIZE);
+	memset(ei->i_data, 0, EXT4_MIN_INLINE_DATA_SIZE);
+
+	if (ext4_has_feature_extents(inode->i_sb) &&
+		(S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) || S_ISLNK(inode->i_mode))) {
+		ext4_set_inode_flag(inode, EXT4_INODE_EXTENTS);
+		ext4_ext_tree_init(handle, inode);
+		struct ext4_ext_path *path = ext4_find_extent(inode, 0, NULL, 0);
+		struct ext4_extent newex;
+
+		newex.ee_block = cpu_to_le32(0);
+		newex.ee_len = cpu_to_le16(len);
+		ext4_ext_store_pblock(&newex, block);
+		ext4_ext_insert_extent(handle, inode, path, &newex, 0);
+	} else {
+		EXT4_I(inode)->i_data[0] = cpu_to_le32(block);
+	}
+	ext4_clear_inode_flag(inode, EXT4_INODE_INLINE_DATA);
+	get_bh(is.iloc.bh);
+	error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
+	EXT4_I(inode)->i_inline_off = 0;
+	EXT4_I(inode)->i_inline_size = 0;
+	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
+out:
+	brelse(is.iloc.bh);
+	if (error == -ENODATA)
+		error = 0;
+	up_write(&ei->i_data_sem);
+	return error;
 }
 
 static int ext4_convert_inline_data_nolock(handle_t *handle,
@@ -1094,8 +1141,9 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
 	int error;
 	void *buf = NULL;
 	struct buffer_head *data_bh = NULL;
-	struct ext4_map_blocks map;
 	int inline_size;
+	ext4_fsblk_t newblock = 0;
+	struct ext4_allocation_request ar;
 
 	inline_size = ext4_get_inline_size(inode);
 	buf = kmalloc(inline_size, GFP_NOFS);
@@ -1120,25 +1168,22 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
 			goto out;
 	}
 
-	error = ext4_destroy_inline_data_nolock(handle, inode);
-	if (error)
-		goto out;
-
-	map.m_lblk = 0;
-	map.m_len = 1;
-	map.m_flags = 0;
-	error = ext4_map_blocks(handle, inode, &map, EXT4_GET_BLOCKS_CREATE);
+	memset(&ar, 0, sizeof(ar));
+	ar.inode = inode;
+	ar.logical = 0;
+	ar.len = 1;
+	if (S_ISREG(inode->i_mode))
+		ar.flags = EXT4_MB_HINT_DATA;
+	else
+		ar.flags = 0;
+	newblock = ext4_mb_new_blocks(handle, &ar, &error);
 	if (error < 0)
-		goto out_restore;
-	if (!(map.m_flags & EXT4_MAP_MAPPED)) {
-		error = -EIO;
-		goto out_restore;
-	}
+		goto out;
 
-	data_bh = sb_getblk(inode->i_sb, map.m_pblk);
+	data_bh = sb_getblk(inode->i_sb, newblock);
 	if (!data_bh) {
 		error = -ENOMEM;
-		goto out_restore;
+		goto out_bh;
 	}
 
 	lock_buffer(data_bh);
@@ -1147,7 +1192,7 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
 	if (error) {
 		unlock_buffer(data_bh);
 		error = -EIO;
-		goto out_restore;
+		goto out_bh;
 	}
 	memset(data_bh->b_data, 0, inode->i_sb->s_blocksize);
 
@@ -1171,12 +1216,14 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
 			error = ext4_mark_inode_dirty(handle, inode);
 	}
 
-out_restore:
 	if (error)
-		ext4_restore_inline_data(handle, inode, iloc, buf, inline_size);
-
-out:
+		goto out_bh;
+	error = ext4_set_inline_data_block(handle, inode, newblock, ar.len);
+out_bh:
+	if (error)
+		ext4_free_blocks(handle, inode, data_bh, newblock, ar.len, EXT4_FREE_BLOCKS_FORGET);
 	brelse(data_bh);
+out:
 	kfree(buf);
 	return error;
 }
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-16 15:02   ` [PATCH v2] " shuo chen
@ 2026-08-16 15:16     ` sashiko-bot
  2026-08-16 20:05     ` [syzbot ci] " syzbot ci
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-16 15:16 UTC (permalink / raw)
  To: shuo chen; +Cc: tytso, linux-ext4

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Dangling extent tree reference causes double-allocation risk in the error path of block conversion.
- [High] The return value of ext4_find_extent() is passed to ext4_ext_insert_extent() without checking for ERR_PTR, leading to a kernel panic on failure.
- [High] The dynamically allocated ext4_ext_path array is leaked, and the return value of ext4_ext_insert_extent() is ignored, suppressing insertion failures.
- [High] If deleting the inline data xattr returns -ENODATA, the function prematurely returns success, skipping extent insertion and leaking the newly allocated file block.
- [High] Destructive modifications to inline data before validating extent tree insertion can lead to unrecoverable data loss.
- [Medium] The commit message claims to modify ext4_readdir, but no such changes are present in the patch.
--

--- Patch [1]: [PATCH v2] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir ---
commit e51d3b5fd4decc28517230e5645f25ee40f9b2fb
Author: shuo chen <1289151713@qq.com>
Subject: Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

The commit rewrites ext4_convert_inline_data_nolock() to allocate blocks
before destroying inline data, aiming for a safer conversion.

Link: https://lore.kernel.org/linux-ext4/tencent_7A0B62235E38C5907A080BE3D56A59C9FD06@qq.com/T/#t> Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

[Severity: Medium]
The commit title mentions adding a check in ext4_readdir(), but the patch
only modifies ext4_convert_inline_data_nolock() in fs/ext4/inline.c. Are
there missing changes, or should the commit message be updated to reflect
the actual modifications?

> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index ceee69a66482b..1fad2a4454ecc 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
[ ... ]
> +static int ext4_set_inline_data_block(handle_t *handle, struct inode *inode,
> +				      ext4_fsblk_t block, unsigned int len)
> +{
[ ... ]
> +	error = ext4_journal_get_write_access(handle, inode->i_sb, is.iloc.bh, EXT4_JTR_NONE);
> +	if (error)
> +		goto out;
> +	error = ext4_xattr_ibody_set(handle, inode, &i, &is);
> +	if (error)
> +		goto out;

[Severity: High]
If the file's inline data fits entirely in i_block, the system data xattr
will not exist, and ext4_xattr_ibody_set() will return -ENODATA. This jumps
to the out label where the error is cleared to 0. Does this cause the extent
insertion to be completely skipped?

If ext4_convert_inline_data_nolock() receives a success code without the
extent being inserted, could it fail to free the newly allocated block and
leak it permanently?

> +	memset((void *)ext4_raw_inode(&is.iloc)->i_block,
> +		0, EXT4_MIN_INLINE_DATA_SIZE);
> +	memset(ei->i_data, 0, EXT4_MIN_INLINE_DATA_SIZE);

[Severity: High]
This irreversibly destroys the existing inline data before we attempt to
insert the new extent into the tree. If ext4_ext_insert_extent() fails, we
return an error and the caller frees the newly allocated block. Doesn't this
mean both the old inline data and the new block are lost, causing
unrecoverable data loss?

> +
> +	if (ext4_has_feature_extents(inode->i_sb) &&
> +		(S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) || S_ISLNK(inode->i_mode))) {
> +		ext4_set_inode_flag(inode, EXT4_INODE_EXTENTS);
> +		ext4_ext_tree_init(handle, inode);
> +		struct ext4_ext_path *path = ext4_find_extent(inode, 0, NULL, 0);

[Severity: High]
Can ext4_find_extent() return an error pointer like ERR_PTR(-ENOMEM)?

Since the returned path is not checked with IS_ERR(), passing it directly to
ext4_ext_insert_extent() could result in a fatal NULL or ERR pointer
dereference.

> +		struct ext4_extent newex;
> +
> +		newex.ee_block = cpu_to_le32(0);
> +		newex.ee_len = cpu_to_le16(len);
> +		ext4_ext_store_pblock(&newex, block);
> +		ext4_ext_insert_extent(handle, inode, path, &newex, 0);

[Severity: High]
It looks like ext4_find_extent() dynamically allocates the path array, but it
is never freed here with ext4_free_ext_path(). Does this leak memory on every
execution?

Additionally, ext4_ext_insert_extent() can fail and return an ERR_PTR or
reallocate the path. By ignoring its return value, we might suppress errors
like -ENOSPC and fail to capture the updated path pointer, leaving the inode
corrupted without its data mapping.

[ ... ]
> @@ -1180,12 +1225,14 @@ static int ext4_convert_inline_data_nolock(handle_t *handle,
>  			error = ext4_mark_inode_dirty(handle, inode);
>  	}
>  
> -out_restore:
>  	if (error)
> -		ext4_restore_inline_data(handle, inode, iloc, buf, inline_size);
> -
> -out:
> +		goto out_bh;
> +	error = ext4_set_inline_data_block(handle, inode, newblock, ar.len);
> +out_bh:
> +	if (error)
> +		ext4_free_blocks(handle, inode, data_bh, newblock, ar.len, EXT4_FREE_BLOCKS_FORGET);

[Severity: Critical]
If ext4_set_inline_data_block() successfully inserts the block into the
extent tree but fails later (for example, in ext4_mark_iloc_dirty()), it
returns an error.

The caller will then execute this path and call ext4_free_blocks() on the
newly allocated block.

Since the insertion into the extent tree is not rolled back, the block
remains mapped in the inode's extent tree but is marked as free in the
filesystem's block bitmap. Could this lead to a double-allocation
vulnerability?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_BDF7C28D876E422418FB57474B765F0C1A09@qq.com?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [syzbot ci] Re: Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
  2026-08-16 15:02   ` [PATCH v2] " shuo chen
  2026-08-16 15:16     ` sashiko-bot
@ 2026-08-16 20:05     ` syzbot ci
  1 sibling, 0 replies; 10+ messages in thread
From: syzbot ci @ 2026-08-16 20:05 UTC (permalink / raw)
  To: 1289151713, ack, adilger.kernel, libaokun, linux-ext4, ojaswin,
	tytso, yi.zhang
  Cc: syzbot, syzkaller-bugs

syzbot ci has tested the following series

[v2] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir
https://lore.kernel.org/all/tencent_BDF7C28D876E422418FB57474B765F0C1A09@qq.com
* [PATCH v2] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

and found the following issue:
WARNING in invalidate_bh_lru

Full report is available here:
https://ci.syzbot.org/series/74cf3860-255d-49bb-b3df-51f8ee44a69f

***

WARNING in invalidate_bh_lru

tree:      linux-next
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next
base:      1351c159c59b04195647917c5a5f0e5467f44bb0
arch:      amd64
compiler:  Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config:    https://ci.syzbot.org/builds/500e2ced-dbe6-480e-affe-813e984ba307/config
syz repro: https://ci.syzbot.org/findings/875c08a7-1950-4014-851b-0d660dfa2413/syz_repro

------------[ cut here ]------------
VFS: brelse: Trying to free free buffer
WARNING: fs/buffer.c:1147 at __brelse fs/buffer.c:1147 [inline], CPU#0: udevd/5048
WARNING: fs/buffer.c:1147 at brelse include/linux/buffer_head.h:326 [inline], CPU#0: udevd/5048
WARNING: fs/buffer.c:1147 at __invalidate_bh_lrus fs/buffer.c:1506 [inline], CPU#0: udevd/5048
WARNING: fs/buffer.c:1147 at invalidate_bh_lru+0xfa/0x1b0 fs/buffer.c:1519, CPU#0: udevd/5048
Modules linked in:
CPU: 0 UID: 0 PID: 5048 Comm: udevd Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:__brelse fs/buffer.c:1147 [inline]
RIP: 0010:brelse include/linux/buffer_head.h:326 [inline]
RIP: 0010:__invalidate_bh_lrus fs/buffer.c:1506 [inline]
RIP: 0010:invalidate_bh_lru+0xfa/0x1b0 fs/buffer.c:1519
Code: f7 be 04 00 00 00 e8 85 c0 d8 ff f0 41 ff 0e eb 1e e8 8a 21 6b ff 80 3c 2b 00 75 20 eb 26 e8 7d 21 6b ff 48 8d 3d e6 bf 01 0e <67> 48 0f b9 3a 4c 89 fd 4f 8d 3c 2c 80 3c 2b 00 74 08 4c 89 ff e8
RSP: 0018:ffffc90000007f38 EFLAGS: 00010006
RAX: ffffffff825be013 RBX: 1ffff11024206b2d RCX: ffff888172139dc0
RDX: 0000000000010000 RSI: 0000000000000000 RDI: ffffffff905da000
RBP: 0000000000000000 R08: ffff8881078c875b R09: 1ffff11020f190eb
R10: dffffc0000000000 R11: ffffed1020f190ec R12: ffff888121035960
R13: 0000000000000008 R14: ffff8881078c8758 R15: dffffc0000000000
FS:  00007f6c32242c80(0000) GS:ffff88818d952000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fc1515eb7c0 CR3: 000000016c5ac000 CR4: 00000000000006f0
Call Trace:
 <IRQ>
 csd_do_func kernel/smp.c:136 [inline]
 __flush_smp_call_function_queue+0x32c/0xa20 kernel/smp.c:580
 __sysvec_call_function_single+0x9a/0x3d0 arch/x86/kernel/smp.c:272
 instr_sysvec_call_function_single arch/x86/kernel/smp.c:267 [inline]
 sysvec_call_function_single+0x9e/0xc0 arch/x86/kernel/smp.c:267
 </IRQ>
 <TASK>
 asm_sysvec_call_function_single+0x1a/0x20 arch/x86/include/asm/idtentry.h:681
RIP: 0010:lock_is_held_type+0x106/0x150 kernel/locking/lockdep.c:5945
Code: 1a 00 00 b8 ff ff ff ff 65 0f c1 05 54 4c 9c 07 83 f8 01 75 25 9c 58 a9 00 02 00 00 75 39 41 f7 c4 00 02 00 00 74 01 fb 89 d8 <5b> 41 5c 41 5d 41 5e 41 5f 5d e9 1b 00 03 00 cc 90 0f 0b 90 48 c7
RSP: 0018:ffffc9000624f818 EFLAGS: 00000206
RAX: 0000000000000001 RBX: 0000000000000001 RCX: 0000000000000046
RDX: 0000000000000000 RSI: ffffffff8e4ae37e RDI: ffffffff8c4bbd80
RBP: 00000000ffffffff R08: ffffc9000020daa7 R09: 1ffff92000041b54
R10: dffffc0000000000 R11: fffff52000041b55 R12: 0000000000000246
R13: ffff888172139dc0 R14: ffffffff8eb59c60 R15: 0000000000000000
 __d_lookup+0x170/0x790 fs/dcache.c:2612
 lookup_fast+0x82/0x5d0 fs/namei.c:1878
 walk_component fs/namei.c:2278 [inline]
 link_path_walk+0x71f/0x1910 fs/namei.c:2656
 path_openat+0x236/0x3830 fs/namei.c:4859
 do_file_open+0x23e/0x4a0 fs/namei.c:4892
 do_sys_openat2+0x115/0x200 fs/open.c:1368
 do_sys_open fs/open.c:1374 [inline]
 __do_sys_openat fs/open.c:1390 [inline]
 __se_sys_openat fs/open.c:1385 [inline]
 __x64_sys_openat+0x138/0x170 fs/open.c:1385
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f6c3231a477
Code: 10 00 00 00 44 8b 54 24 e0 48 89 44 24 c0 48 8d 44 24 d0 48 89 44 24 c8 44 89 c2 4c 89 ce bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 76 10 48 8b 15 82 69 0d 00 f7 d8 64 89 02 48 83
RSP: 002b:00007ffe8da39558 EFLAGS: 00000287 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 000055a56911c8f0 RCX: 00007f6c3231a477
RDX: 0000000000090800 RSI: 000055a569100840 RDI: 00000000ffffff9c
RBP: 000055a569196980 R08: 0000000000090800 R09: 000055a569100840
R10: 0000000000000000 R11: 0000000000000287 R12: 000055a569100840
R13: 00000000000000ff R14: 000055a53ac761c4 R15: 0000000000000000
 </TASK>
----------------
Code disassembly (best guess):
   0:	f7 be 04 00 00 00    	idivl  0x4(%rsi)
   6:	e8 85 c0 d8 ff       	call   0xffd8c090
   b:	f0 41 ff 0e          	lock decl (%r14)
   f:	eb 1e                	jmp    0x2f
  11:	e8 8a 21 6b ff       	call   0xff6b21a0
  16:	80 3c 2b 00          	cmpb   $0x0,(%rbx,%rbp,1)
  1a:	75 20                	jne    0x3c
  1c:	eb 26                	jmp    0x44
  1e:	e8 7d 21 6b ff       	call   0xff6b21a0
  23:	48 8d 3d e6 bf 01 0e 	lea    0xe01bfe6(%rip),%rdi        # 0xe01c010
* 2a:	67 48 0f b9 3a       	ud1    (%edx),%rdi <-- trapping instruction
  2f:	4c 89 fd             	mov    %r15,%rbp
  32:	4f 8d 3c 2c          	lea    (%r12,%r13,1),%r15
  36:	80 3c 2b 00          	cmpb   $0x0,(%rbx,%rbp,1)
  3a:	74 08                	je     0x44
  3c:	4c 89 ff             	mov    %r15,%rdi
  3f:	e8                   	.byte 0xe8


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a fix for this bug, please reply with `#syz test`
(on a separate line) and attach the patch to the email.

Notes:
- The patch will be applied on top of the tested series (as an
  incremental fix).
- To test a new version of the whole series, please send it directly
  to syzbot@lists.linux.dev.
- Arguments like custom git repos and branches are not supported.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-16 20:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  8:33 [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir pipishuo
2026-08-13  8:49 ` sashiko-bot
2026-08-14  1:27   ` shuo chen
2026-08-14  3:31 ` Theodore Tso
2026-08-14  8:40   ` shuo chen
2026-08-14 13:57     ` Theodore Tso
2026-08-15  1:43       ` shuo chen
2026-08-16 15:02   ` [PATCH v2] " shuo chen
2026-08-16 15:16     ` sashiko-bot
2026-08-16 20:05     ` [syzbot ci] " syzbot ci

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.