[PATCH v2] ext4: validate ea_ino and size in check_xattrs

[PATCH v2] ext4: validate ea_ino and size in check_xattrs

[Deepanshu Kartikey]

During xattr block validation, check_xattrs() processes xattr entries
without validating that entries claiming to use EA inodes have non-zero
sizes. Corrupted filesystems may contain xattr entries where e_value_size
is zero but e_value_inum is non-zero, indicating invalid xattr data.

Add validation in check_xattrs() to detect this corruption pattern early
and return -EFSCORRUPTED, preventing invalid xattr entries from causing
issues throughout the ext4 codebase.

Suggested-by: Theodore Ts'o <tytso@mit.edu>
Reported-by: syzbot+4c9d23743a2409b80293@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=4c9d23743a2409b80293
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
---
Changes in v2:
- Moved validation from ext4_xattr_move_to_block() to check_xattrs() as suggested by Theodore Ts'o
- This provides broader coverage and may address other similar syzbot reports

 fs/ext4/xattr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 5a6fe1513fd2..d621e77c8c4d 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -251,6 +251,10 @@ check_xattrs(struct inode *inode, struct buffer_head *bh,
 			err_str = "invalid ea_ino";
 			goto errout;
 		}
+		if (ea_ino && !size) {
+			err_str = "invalid size in ea xattr";
+			goto errout;
+		}
 		if (size > EXT4_XATTR_SIZE_MAX) {
 			err_str = "e_value size too large";
 			goto errout;
-- 
2.43.0

Re: [PATCH v2] ext4: validate ea_ino and size in check_xattrs

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]

On Sep 23, 2025, at 7:32 AM, Deepanshu Kartikey <kartikey406@gmail.com> wrote:
> 
> During xattr block validation, check_xattrs() processes xattr entries
> without validating that entries claiming to use EA inodes have non-zero
> sizes. Corrupted filesystems may contain xattr entries where e_value_size
> is zero but e_value_inum is non-zero, indicating invalid xattr data.
> 
> Add validation in check_xattrs() to detect this corruption pattern early
> and return -EFSCORRUPTED, preventing invalid xattr entries from causing
> issues throughout the ext4 codebase.

This should also have a corresponding check and fix in e2fsck, otherwise
the kernel will fail but there is no way to repair such a filesystem.

Cheers, Andreas

> Suggested-by: Theodore Ts'o <tytso@mit.edu>
> Reported-by: syzbot+4c9d23743a2409b80293@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=4c9d23743a2409b80293
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> ---
> Changes in v2:
> - Moved validation from ext4_xattr_move_to_block() to check_xattrs() as suggested by Theodore Ts'o
> - This provides broader coverage and may address other similar syzbot reports
> 
> fs/ext4/xattr.c | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 5a6fe1513fd2..d621e77c8c4d 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -251,6 +251,10 @@ check_xattrs(struct inode *inode, struct buffer_head *bh,
> 			err_str = "invalid ea_ino";
> 			goto errout;
> 		}
> +		if (ea_ino && !size) {
> +			err_str = "invalid size in ea xattr";
> +			goto errout;
> +		}
> 		if (size > EXT4_XATTR_SIZE_MAX) {
> 			err_str = "e_value size too large";
> 			goto errout;
> --
> 2.43.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH v2] ext4: validate ea_ino and size in check_xattrs

[Deepanshu Kartikey]

Thank you for the feedback on the e2fsck coordination.

You raise a valid point about the complete repair workflow. I'm happy to work on a corresponding e2fsck patch if that would be helpful, though I'd appreciate guidance on the preferred approach:

1. Should I proceed with the kernel patch first and then work on e2fsck, or would you prefer coordinated patches?

2. For the e2fsck side, would the appropriate fix be to:
   - Clear e_value_inum when e_value_size is zero, or
   - Remove the entire corrupted xattr entry?

I'm new to e2fsprogs development but willing to learn the codebase if you think it's valuable to have matching fixes. Alternatively, if there are others typically handling the e2fsck side of ext4 corruption fixes, I'm happy to focus on the kernel patch and coordinate with them.

Thanks for considering the broader user experience - I hadn't fully thought through the repair workflow.

Deepanshu Kartikey

Re: [PATCH v2] ext4: validate ea_ino and size in check_xattrs

[Theodore Ts'o]

[-- Attachment #1: Type: text/plain, Size: 5889 bytes --]

On Tue, Sep 23, 2025 at 12:04:42PM -0600, Andreas Dilger wrote:
> On Sep 23, 2025, at 7:32 AM, Deepanshu Kartikey <kartikey406@gmail.com> wrote:
> > 
> > During xattr block validation, check_xattrs() processes xattr entries
> > without validating that entries claiming to use EA inodes have non-zero
> > sizes. Corrupted filesystems may contain xattr entries where e_value_size
> > is zero but e_value_inum is non-zero, indicating invalid xattr data.
> > 
> > Add validation in check_xattrs() to detect this corruption pattern early
> > and return -EFSCORRUPTED, preventing invalid xattr entries from causing
> > issues throughout the ext4 codebase.
> 
> This should also have a corresponding check and fix in e2fsck, otherwise
> the kernel will fail but there is no way to repair such a filesystem.

Yep, I've checked and e2fsprogs doesn't handle this case correctly.

Patch attached....

						- Ted

From 003ead91bbedd39915ea7e8cd75c4278932504a1 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Thu, 25 Sep 2025 21:11:52 -0400
Subject: [PATCH] e2fsck: check for extended attributes with ea_inode but a
 zero ea_size

The combination of e_value_inum != 0 and e_value_size == 0 is invalid
and can trigger kernel warnings.  This should only happen with
delierately corrupted extended attribute entries; so if we come across
one, just clear the xattrs.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 e2fsck/pass1.c                |   6 ++++++
 tests/f_ea_zero_size/expect.1 |  30 ++++++++++++++++++++++++++++++
 tests/f_ea_zero_size/expect.2 |   7 +++++++
 tests/f_ea_zero_size/image.gz | Bin 0 -> 1313 bytes
 tests/f_ea_zero_size/name     |   1 +
 5 files changed, 44 insertions(+)
 create mode 100644 tests/f_ea_zero_size/expect.1
 create mode 100644 tests/f_ea_zero_size/expect.2
 create mode 100644 tests/f_ea_zero_size/image.gz
 create mode 100644 tests/f_ea_zero_size/name

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index e7d5d0ae9..fdde76cc2 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -343,6 +343,12 @@ static problem_t check_large_ea_inode(e2fsck_t ctx,
 
 	e2fsck_read_inode(ctx, entry->e_value_inum, &inode, "pass1");
 
+	if (entry->e_value_size == 0 ||
+	    entry->e_value_size != EXT2_I_SIZE(&inode)) {
+		pctx->num = entry->e_value_size;
+		return PR_1_ATTR_VALUE_SIZE;
+	}
+
 	retval = ext2fs_ext_attr_hash_entry3(ctx->fs, entry, NULL, &hash,
 					     &signed_hash);
 	if (retval) {
diff --git a/tests/f_ea_zero_size/expect.1 b/tests/f_ea_zero_size/expect.1
new file mode 100644
index 000000000..2aa0ae653
--- /dev/null
+++ b/tests/f_ea_zero_size/expect.1
@@ -0,0 +1,30 @@
+Pass 1: Checking inodes, blocks, and sizes
+Extended attribute in inode 12 has a value size (0) which is invalid
+Clear? yes
+
+Inode 12, i_blocks is 8, should be 0.  Fix? yes
+
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Regular filesystem inode 14 has EA_INODE flag set. Clear? yes
+
+Unattached inode 14
+Connect to /lost+found? yes
+
+Inode 14 ref count is 2, should be 1.  Fix? yes
+
+Pass 5: Checking group summary information
+Block bitmap differences:  -13
+Fix? yes
+
+Free blocks count wrong for group #0 (46, counted=47).
+Fix? yes
+
+Free blocks count wrong (46, counted=47).
+Fix? yes
+
+
+test_filesys: ***** FILE SYSTEM WAS MODIFIED *****
+test_filesys: 21/32 files (0.0% non-contiguous), 17/64 blocks
+Exit status is 1
diff --git a/tests/f_ea_zero_size/expect.2 b/tests/f_ea_zero_size/expect.2
new file mode 100644
index 000000000..17211daf9
--- /dev/null
+++ b/tests/f_ea_zero_size/expect.2
@@ -0,0 +1,7 @@
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+test_filesys: 21/32 files (0.0% non-contiguous), 17/64 blocks
+Exit status is 0
diff --git a/tests/f_ea_zero_size/image.gz b/tests/f_ea_zero_size/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..fff5b203f8fa3d36da038fd8bae5a1198fa91db4
GIT binary patch
literal 1313
zcmc&y{a2EA9DUR_Yt_NEPBt~HvsG(R&dN&LiBiugr>Liu<on8*qA6-^zSQ<)%~taj
zIfBLpGd(#iH8rJTfE7g=8@^0%prRsRJ}5yxBJjv}vOi(J?Edh%pU*j;d+z<=dKa%;
zS)dzAb#|d-oJmQBW=Eye^{2d`H#NI91cpYtIp5rP^XB!FvB$$OTJ@Ey!W)IZxdz@Y
z9DQ(v-cFjv)p}hKu_~rZgZ%uq%p5JXx@Y;)SlbGao@=DfrjM$hi>zaM0wEOc*pNZw
zO!27So>SBq1l(2wDWxOC`QAxokbYJFy4#0>V+DeWV>tqkY+-TLwI8<&fibu;jKnv_
z-l{(nkLI7c#munK8-k2j(~QBck;vr>d>-KYkT+Q;Miy;iPic~_r?>CVCv20j261oj
zPJMkSYy3$$ENC+}M7-dsEnPZgR}~JQX;0@ZY10R$Q=4X;p!@UhS%%?L`|Z@bWfmt0
zx{{on*0j~fv1yPk7hnj2PHWT5P9mvLc6RG1$WlLg6M0VyH=Zb+VKBZXMYV*)JX0%F
zgZjPe{ETW|GwGmx!HpS9RC?7#;pV<@Vm!61o|u?nQ><X00c>b(p2!af0^|tb8mde4
z0{WUvjQmo?8cO&z8~cDsK0v`P_^1GRw`8B4L@fgY1TfWwxt}%Yqzu^~m0(eIDUr8F
z0m4f1u+w9Tx-5RhVfL=sUimy))HzY#u8Nk((WqCncY3X0vKdn%1xT9xkO?z*fi9oT
z;O`muGI&Hze_y(V5o$G4X_nl5J3%Dqd}+dIPH;i7Wkh#Y6J=8=>KBL)lk}~V1WS25
zoQ@82>gc|c#w35vLZU<o>qNyWnF6L1rlBhTeiI3^gjT&UgEIYI<KPgm=4*-TO`K-C
zcpF<XdCkqZn4RB<2=|+tr9{e!C7hH7G&t`}sp7~D0*I6u2JzwJ^3Ad@UpMxA<nk;i
z-bMU4yy{c&C4jS(OR4}uf4=A;x*;x4UOLQrwrYgKMYmtgX>34(DQ|-1ZT10UOK*3i
z{3}CW`~sY0I3Jpl2KJ(`r#8QYtE0pCXnSp3_$-)@vxpI5$CR+PrB&%Zk^qWZZI4V;
z*`iJoXYrmLj^Vw_&~Or0-$g;}i~=0{g^Kssftvjco0dzk^VsiQ{p|XpIo7M9z}#FE
zKESH%EV36qsTG&4T|+KtlG}a?Nmmd*KX|c;Td-7DJ;FSvHeNpbaEC;O=yaYPzIN@!
zJO0QWH|5hcDUbb0Kd+d-r>gWYxIj?%C?&lbfHpw@a9y@y)ZAt+6C?r4Eri}WD(Jv?
zf`{W~@E4a9MjL-nAJaX3T#Xh64)@V~hy(18HDeDm3E!iM1N0>ctv`o@eBw~SZ<l8`
z|D^N3TB{8tS|4CT7tJ6g5$A>v*pBDR@&Md8ZwfI3N0!N5&oQCnnQ*D4eVP6P`r?WZ
zN?u8uO+VL>dw#8Fz+|9r2%ZvAnL~|}tJaaxA2xpIzb9ty?i_cw%20H_k2!0O?^3n6
fwTiy^E+HeTg$6cvIwJg!_zQD6Iv~q~pcT+RmcD}-

literal 0
HcmV?d00001

diff --git a/tests/f_ea_zero_size/name b/tests/f_ea_zero_size/name
new file mode 100644
index 000000000..95d9893c1
--- /dev/null
+++ b/tests/f_ea_zero_size/name
@@ -0,0 +1 @@
+zero extended attribute size with ea_inode
-- 
2.51.0



[-- Attachment #2: foo.img.gz --]
[-- Type: application/gzip, Size: 1328 bytes --]

Re: [PATCH v2] ext4: validate ea_ino and size in check_xattrs

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 4824 bytes --]

On Sep 23, 2025, at 7:10 PM, Deepanshu Kartikey <kartikey406@gmail.com> wrote:
> 
> Thank you for the feedback on the e2fsck coordination.
> 
> You raise a valid point about the complete repair workflow. I'm happy
> to work on a corresponding e2fsck patch if that would be helpful,
> though I'd appreciate guidance on the preferred approach:
> 
> 	• Should I proceed with the kernel patch first and then work
>           on e2fsck, or would you prefer coordinated patches?

Coordinated patches would be best.  My concern is that the check in
this patch is essentially causing the filesystem to be aborted, then
there may be no way to repair it before remount, repeat.

*NOTE* I haven't tested whether e2fsck already handles this scenario
correctly, but it is definitely worthwhile to test this with your
reproducer image to see if e2fsck already fixes the issue. If that is
already the case, then there is nothing more to be done.

A quick look through the users of e_value_inum in the e2fsck code
doesn't show anywhere that is checking e_value_size > 0, but the
e2fsprogs xattr  handling is spread over a few different parts of
the code so I might have missed something.

If e2fsck does *not* repair this error, then the right workflow is to
make a *minimal* filesystem image with this corruption and use it for
a new test case.  There is a "make testnew" target in tests/ that
creates a "f_testnew" subdirectory with an image file, then you can
mount this with loopback and/or use debugfs to create a large xattr
inode and zero e_value_size (not sure if in xattr header or the xattr
inode size, or maybe do some combinations of inconsistencies.

> 	• For the e2fsck side, would the appropriate fix be to:
> 		• Clear e_value_inum when e_value_size is zero, or
> 		• Remove the entire corrupted xattr entry?

Probably there is no benefit to remove e_value_inum and have an xattr without
any value, so it would be best to just remove this xattr completely.  The ext4
code should never (AFAIK) create a zero-length xattr in an external inode, as
this is totally pointless.

> I'm new to e2fsprogs development but willing to learn the codebase if you
> think it's valuable to have matching fixes. Alternatively, if there are
> others typically handling the e2fsck side of ext4 corruption fixes, I'm
> happy to focus on the kernel patch and coordinate with them.

The ext4 developers are typically handling both sides of the story together.

Cheers, Andreas

> Thanks for considering the broader user experience - I hadn't fully thought
> through the repair workflow.
> 
> 
> On Tue, Sep 23, 2025 at 11:34 PM Andreas Dilger <adilger@dilger.ca> wrote:
> On Sep 23, 2025, at 7:32 AM, Deepanshu Kartikey <kartikey406@gmail.com> wrote:
> >
> > During xattr block validation, check_xattrs() processes xattr entries
> > without validating that entries claiming to use EA inodes have non-zero
> > sizes. Corrupted filesystems may contain xattr entries where e_value_size
> > is zero but e_value_inum is non-zero, indicating invalid xattr data.
> >
> > Add validation in check_xattrs() to detect this corruption pattern early
> > and return -EFSCORRUPTED, preventing invalid xattr entries from causing
> > issues throughout the ext4 codebase.
> 
> This should also have a corresponding check and fix in e2fsck, otherwise
> the kernel will fail but there is no way to repair such a filesystem.
> 
> Cheers, Andreas
> 
> > Suggested-by: Theodore Ts'o <tytso@mit.edu>
> > Reported-by: syzbot+4c9d23743a2409b80293@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=4c9d23743a2409b80293
> > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > ---
> > ---
> > Changes in v2:
> > - Moved validation from ext4_xattr_move_to_block() to check_xattrs() as suggested by Theodore Ts'o
> > - This provides broader coverage and may address other similar syzbot reports
> >
> > fs/ext4/xattr.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> > index 5a6fe1513fd2..d621e77c8c4d 100644
> > --- a/fs/ext4/xattr.c
> > +++ b/fs/ext4/xattr.c
> > @@ -251,6 +251,10 @@ check_xattrs(struct inode *inode, struct buffer_head *bh,
> >                       err_str = "invalid ea_ino";
> >                       goto errout;
> >               }
> > +             if (ea_ino && !size) {
> > +                     err_str = "invalid size in ea xattr";
> > +                     goto errout;
> > +             }
> >               if (size > EXT4_XATTR_SIZE_MAX) {
> >                       err_str = "e_value size too large";
> >                       goto errout;
> > --
> > 2.43.0
> >
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH v2] ext4: validate ea_ino and size in check_xattrs

[Theodore Ts'o]

On Fri, Sep 26, 2025 at 01:47:14PM -0600, Andreas Dilger wrote:
> 
> *NOTE* I haven't tested whether e2fsck already handles this scenario
> correctly, but it is definitely worthwhile to test this with your
> reproducer image to see if e2fsck already fixes the issue. If that is
> already the case, then there is nothing more to be done.

It doesn't.  But see the patch that I sent to fix this.

> If e2fsck does *not* repair this error, then the right workflow is to
> make a *minimal* filesystem image with this corruption and use it for
> a new test case.

I aready sent a patch on this thread, and it includes a minimal file
sytem image.  Unfortunately, we don't have easy way to create
corrupted extended attributre entries using the debugfs tool.  This is
why I decided to just create the patch and test case, instead of
asking Deepanshu to try to create it, since creating the test case
requires using a hex editor and understanding of the extended
attribute layout.  One of these days we really should add the ability
to easily edit extended attribute blocks to corrupt them, but to date
it's been easier for me to just use emacs hexl-mode to edit the image.

The good news is that there are tools to examine extended attributes.
For example:

% debugfs /tmp/f_ea_zero_size.img
debugfs 1.47.3-rc2 (12-Jun-2025)
debugfs:  stat lustre
Inode: 12   Type: regular    Mode:  0644   Flags: 0x80000
Generation: 1631366467    Version: 0x00000000:00000001
User:     0   Group:     0   Project:     0   Size: 0
File ACL: 13
Links: 1   Blockcount: 8
Fragment:  Address: 0    Number: 0    Size: 0
 ctime: 0x594f621c:5143fea0 -- Sun Jun 25 03:11:24 2017
 atime: 0x594f621c:396c7aa4 -- Sun Jun 25 03:11:24 2017
 mtime: 0x594f621c:396c7aa4 -- Sun Jun 25 03:11:24 2017
crtime: 0x594f621c:396c7aa4 -- Sun Jun 25 03:11:24 2017
Size of extra inode fields: 32
EXTENTS:
debugfs:  block_dump -x 13
magic = ea020000, length = 4096
refcount = 1, blocks = 1
hash = 767a7676, checksum = 00000000
reserved: 00000000 00000000 00000000

offset = 32 (0040), hash = 3109, name_len = 2, name_index = 1
value_offset = 0 (0000), value_inum = 14, value_size = 0
name = be

offset = 52 (0064), hash = 2053076598, name_len = 2, name_index = 1
value_offset = 3996 (7634), value_inum = 0, value_size = 100
name = bi
value = vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv

last entry found at offset 72 (0110)

There are also debugfs commands "ea_get, ea_set, and ea_list", which
is good for edit valid extended attribute blocks.  So what I tend to
do is to use these tools to create a valid extended attribute block
--- and then I'll corrupt it using emacs hexl-mode.

    	     	  	     	   	 - Ted

Re: [PATCH v2] ext4: validate ea_ino and size in check_xattrs

[Theodore Ts'o]

On Tue, 23 Sep 2025 19:02:45 +0530, Deepanshu Kartikey wrote:
> During xattr block validation, check_xattrs() processes xattr entries
> without validating that entries claiming to use EA inodes have non-zero
> sizes. Corrupted filesystems may contain xattr entries where e_value_size
> is zero but e_value_inum is non-zero, indicating invalid xattr data.
> 
> Add validation in check_xattrs() to detect this corruption pattern early
> and return -EFSCORRUPTED, preventing invalid xattr entries from causing
> issues throughout the ext4 codebase.
> 
> [...]

Applied, thanks!

[1/1] ext4: validate ea_ino and size in check_xattrs
      commit: 44d2a72f4d64655f906ba47a5e108733f59e6f28

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>