linux-ext4 - Re: [PATCH v2] ext4: validate ea_ino and size in check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250926011530.GH19231@mit.edu>
Date: Thu, 25 Sep 2025 21:15:30 -0400
From: "Theodore Ts'o" <tytso@....edu>
To: Andreas Dilger <adilger@...ger.ca>
Cc: Deepanshu Kartikey <kartikey406@...il.com>,
        linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH v2] ext4: validate ea_ino and size in check_xattrs

On Tue, Sep 23, 2025 at 12:04:42PM -0600, Andreas Dilger wrote:
> On Sep 23, 2025, at 7:32 AM, Deepanshu Kartikey <kartikey406@...il.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@....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@....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@...prRsRJ}5yxBJjv}vOi(J?Edh%pU*j;d+z<=dKa%;
zS)dzAb#|d-oJmQBW=Eye^{2d`H#NI91cpYtIp5rP^XB!FvB$$OTJ@...W)IZxdz@Y
z9DQ(v-cFjv)p}hKu_~rZgZ%uq%p5JXx@Y;)SlbGao@...rjM$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@...0R$Q=4X;p!@...%%?L`|Z@...mt0
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@...1TfWwxt}%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@...xA<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~@...9MjL-nAJaX3T#Xh64)@V~hy(18HDeDm3E!iM1N0>ctv`o@...~SZ<l8`
z|D^N3TB{8tS|4CT7tJ6g5$A>v*pBDR@...8ZwfI3N0!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



Download attachment "foo.img.gz" of type "application/gzip" (1328 bytes)