* [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; 4+ 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] 4+ 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; 4+ 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] 4+ 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; 4+ 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] 4+ 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
1 sibling, 0 replies; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2026-08-14 3:32 UTC | newest]
Thread overview: 4+ 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
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.