* [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
@ 2026-09-09 1:12 shuo chen
2026-09-09 1:24 ` sashiko-bot
2026-09-09 10:37 ` Jan Kara
0 siblings, 2 replies; 6+ messages in thread
From: shuo chen @ 2026-09-09 1:12 UTC (permalink / raw)
To: tytso
Cc: adilger.kernel, libaokun, jack, ojaswin, yi.zhang, linux-ext4,
shuo chen
If a file has EXT4_INODE_INLINE_DATA set but lacks the
EXT4_STATE_XATTR flag, ext4_xattr_ibody_find() will exit early without
modifying is.s.not_found. This may cause ext4_xattr_ibody_set()
to behave incorrectly.
Signed-off-by: shuo chen <1289151713@qq.com>
---
fs/ext4/inline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index ceee69a66482..3398208f3e66 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -437,7 +437,7 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
{
struct ext4_inode_info *ei = EXT4_I(inode);
struct ext4_xattr_ibody_find is = {
- .s = { .not_found = 0, },
+ .s = { .not_found = -ENODATA, },
};
struct ext4_xattr_info i = {
.name_index = EXT4_XATTR_INDEX_SYSTEM,
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
2026-09-09 1:12 [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock shuo chen
@ 2026-09-09 1:24 ` sashiko-bot
2026-09-09 2:56 ` shuo chen
2026-09-09 10:37 ` Jan Kara
1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-09-09 1:24 UTC (permalink / raw)
To: shuo chen; +Cc: linux-ext4, tytso
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The patch initializes `is.s.not_found` to `-ENODATA` but fails to check this value before unconditionally deleting the inline data xattr, leading to a catastrophic size_t underflow and KASAN out-of-bounds write (kernel panic).
--
commit af434a75deba4ee29e45b41767340323974fc530
Author: shuo chen <1289151713@qq.com>
ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
If a file has EXT4_INODE_INLINE_DATA set but lacks the EXT4_STATE_XATTR
flag, ext4_xattr_ibody_find() will exit early without modifying
is.s.not_found. This may cause ext4_xattr_ibody_set() to behave
incorrectly.
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index caa51473207d7..4f02e7e9f5a30 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -405,7 +405,7 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
> {
> struct ext4_inode_info *ei = EXT4_I(inode);
> struct ext4_xattr_ibody_find is = {
> - .s = { .not_found = 0, },
> + .s = { .not_found = -ENODATA, },
[Severity: Critical]
This is a pre-existing issue, but does this patch fully resolve the problem?
While is.s.not_found is now correctly initialized to -ENODATA,
ext4_destroy_inline_data_nolock() still unconditionally calls
ext4_xattr_ibody_inline_set() without checking if is.s.not_found indicates
the attribute is missing.
When ext4_xattr_ibody_find() returns 0 and leaves not_found as -ENODATA,
this leads to a deletion operation (i.value == NULL) in
ext4_xattr_set_entry() on a non-existent entry.
Because the deletion path in ext4_xattr_set_entry() ignores s->not_found:
fs/ext4/xattr.c:ext4_xattr_set_entry() {
...
if (!i->value) {
/* Remove old name. */
size_t size = EXT4_XATTR_LEN(name_len);
last = ENTRY((void *)last - size);
memmove(here, (void *)here + size,
(void *)last - (void *)here + sizeof(__u32));
memset(last, 0, size);
}
...
}
Executing this when the entry doesn't exist causes an integer underflow in the
memmove() length parameter, which leads to an out-of-bounds write and kernel
panic.
Should ext4_destroy_inline_data_nolock() explicitly check is.s.not_found
and skip ext4_xattr_ibody_inline_set() when it is -ENODATA?
> };
> struct ext4_xattr_info i = {
> .name_index = EXT4_XATTR_INDEX_SYSTEM,
--
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_CD1AF9C3A7875C42F2187B57D74A1C5EE609@qq.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
2026-09-09 1:24 ` sashiko-bot
@ 2026-09-09 2:56 ` shuo chen
0 siblings, 0 replies; 6+ messages in thread
From: shuo chen @ 2026-09-09 2:56 UTC (permalink / raw)
To: sashiko-reviews; +Cc: tytso, linux-ext4
On Wed, Sep 09, 2026 at 01:24:42AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] The patch initializes `is.s.not_found` to `-ENODATA` but fails to check this value before unconditionally deleting the inline data xattr, leading to a catastrophic size_t underflow and KASAN out-of-bounds write (kernel panic).
> --
>
> commit af434a75deba4ee29e45b41767340323974fc530
> Author: shuo chen <1289151713@qq.com>
>
> ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
>
> If a file has EXT4_INODE_INLINE_DATA set but lacks the EXT4_STATE_XATTR
> flag, ext4_xattr_ibody_find() will exit early without modifying
> is.s.not_found. This may cause ext4_xattr_ibody_set() to behave
> incorrectly.
>
> > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> > index caa51473207d7..4f02e7e9f5a30 100644
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -405,7 +405,7 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
> > {
> > struct ext4_inode_info *ei = EXT4_I(inode);
> > struct ext4_xattr_ibody_find is = {
> > - .s = { .not_found = 0, },
> > + .s = { .not_found = -ENODATA, },
>
> [Severity: Critical]
> This is a pre-existing issue, but does this patch fully resolve the problem?
>
> While is.s.not_found is now correctly initialized to -ENODATA,
> ext4_destroy_inline_data_nolock() still unconditionally calls
> ext4_xattr_ibody_inline_set() without checking if is.s.not_found indicates
> the attribute is missing.
>
> When ext4_xattr_ibody_find() returns 0 and leaves not_found as -ENODATA,
> this leads to a deletion operation (i.value == NULL) in
> ext4_xattr_set_entry() on a non-existent entry.
>
> Because the deletion path in ext4_xattr_set_entry() ignores s->not_found:
>
> fs/ext4/xattr.c:ext4_xattr_set_entry() {
> ...
> if (!i->value) {
> /* Remove old name. */
> size_t size = EXT4_XATTR_LEN(name_len);
>
> last = ENTRY((void *)last - size);
> memmove(here, (void *)here + size,
> (void *)last - (void *)here + sizeof(__u32));
> memset(last, 0, size);
> }
> ...
> }
>
> Executing this when the entry doesn't exist causes an integer underflow in the
> memmove() length parameter, which leads to an out-of-bounds write and kernel
> panic.
>
> Should ext4_destroy_inline_data_nolock() explicitly check is.s.not_found
> and skip ext4_xattr_ibody_inline_set() when it is -ENODATA?
Got it. I will add checks and send v2.
>
> > };
> > struct ext4_xattr_info i = {
> > .name_index = EXT4_XATTR_INDEX_SYSTEM,
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/tencent_CD1AF9C3A7875C42F2187B57D74A1C5EE609@qq.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
2026-09-09 1:12 [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock shuo chen
2026-09-09 1:24 ` sashiko-bot
@ 2026-09-09 10:37 ` Jan Kara
2026-09-09 11:30 ` shuo chen
1 sibling, 1 reply; 6+ messages in thread
From: Jan Kara @ 2026-09-09 10:37 UTC (permalink / raw)
To: shuo chen
Cc: tytso, adilger.kernel, libaokun, jack, ojaswin, yi.zhang,
linux-ext4
On Wed 09-09-26 09:12:35, shuo chen wrote:
> If a file has EXT4_INODE_INLINE_DATA set but lacks the
> EXT4_STATE_XATTR flag,
But how can this happen? Inline data is stored in xattrs so it shouldn't be
possible...
Honza
> ext4_xattr_ibody_find() will exit early without
> modifying is.s.not_found. This may cause ext4_xattr_ibody_set()
> to behave incorrectly.
>
> Signed-off-by: shuo chen <1289151713@qq.com>
> ---
> fs/ext4/inline.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index ceee69a66482..3398208f3e66 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -437,7 +437,7 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
> {
> struct ext4_inode_info *ei = EXT4_I(inode);
> struct ext4_xattr_ibody_find is = {
> - .s = { .not_found = 0, },
> + .s = { .not_found = -ENODATA, },
> };
> struct ext4_xattr_info i = {
> .name_index = EXT4_XATTR_INDEX_SYSTEM,
> --
> 2.55.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
2026-09-09 10:37 ` Jan Kara
@ 2026-09-09 11:30 ` shuo chen
2026-09-09 12:28 ` Jan Kara
0 siblings, 1 reply; 6+ messages in thread
From: shuo chen @ 2026-09-09 11:30 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-ext4
On Wed, Sep 09, 2026 at 12:37:50PM +0200, Jan Kara wrote:
> On Wed 09-09-26 09:12:35, shuo chen wrote:
> > If a file has EXT4_INODE_INLINE_DATA set but lacks the
> > EXT4_STATE_XATTR flag,
>
> But how can this happen? Inline data is stored in xattrs so it shouldn't be
> possible...
Thank you for your guidance.
when finding xattr, there is no space left. Can this happen?
int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
struct ext4_xattr_ibody_find *is)
{
......
if (!EXT4_INODE_HAS_XATTR_SPACE(inode))
return 0;
......
}
If this can happen, it makes sense to fix the initial value.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock
2026-09-09 11:30 ` shuo chen
@ 2026-09-09 12:28 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2026-09-09 12:28 UTC (permalink / raw)
To: shuo chen; +Cc: Jan Kara, linux-ext4
On Wed 09-09-26 19:30:24, shuo chen wrote:
> On Wed, Sep 09, 2026 at 12:37:50PM +0200, Jan Kara wrote:
> > On Wed 09-09-26 09:12:35, shuo chen wrote:
> > > If a file has EXT4_INODE_INLINE_DATA set but lacks the
> > > EXT4_STATE_XATTR flag,
> >
> > But how can this happen? Inline data is stored in xattrs so it shouldn't be
> > possible...
> Thank you for your guidance.
> when finding xattr, there is no space left. Can this happen?
> int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
> struct ext4_xattr_ibody_find *is)
> {
> ......
> if (!EXT4_INODE_HAS_XATTR_SPACE(inode))
> return 0;
> ......
> }
> If this can happen, it makes sense to fix the initial value.
Let me repeat in more detail because you still don't seem to follow.
EXT4_STATE_XATTR means inode has some extended attributes.
EXT4_INODE_INLINE_DATA means inode has inline data. Inline data is a
special type of an extended attribute. Hence it should be impossible that
EXT4_INODE_INLINE_DATA is set while EXT4_STATE_XATTR is not set. If you can
find a code path where this happens, do tell. Otherwise I'm not spending
more time on this.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-09 12:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 1:12 [PATCH] ext4:Fix the initial value of not_found in ext4_destroy_inline_data_nolock shuo chen
2026-09-09 1:24 ` sashiko-bot
2026-09-09 2:56 ` shuo chen
2026-09-09 10:37 ` Jan Kara
2026-09-09 11:30 ` shuo chen
2026-09-09 12:28 ` Jan Kara
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.