[PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

[PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

[pipishuo]

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

Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

[sashiko-bot]

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

Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

[shuo chen]

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

Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

[Theodore Tso]

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

Re: [PATCH] Add EXT4_STATE_MAY_INLINE_DATA check in ext4_readdir

[shuo chen]

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