diff mbox series

jffs2: Decrease xattr length limit to avoid summary write error

Message ID 20250423193209.5811-1-arapovl839@gmail.com
State New
Headers show
Series jffs2: Decrease xattr length limit to avoid summary write error | expand

Commit Message

Leonid Arapov April 23, 2025, 7:32 p.m. UTC
When fuzzing, the following error is observed:

jffs2: warning: (1096) jffs2_sum_write_sumnode: Empty summary info!!!
------------[ cut here ]------------
kernel BUG at fs/jffs2/summary.c:865!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI
CPU: 1 PID: 1096 Comm: syz-executor340 Not tainted
6.1.108-syzkaller-00007-g86fb5a1a71c9 #0
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 
04/01/2014
RIP: 0010:jffs2_sum_write_sumnode.cold+0x195/0x43b fs/jffs2/summary.c:865
Call Trace:
 jffs2_do_reserve_space+0xa18/0xd60 fs/jffs2/nodemgmt.c:388
 jffs2_reserve_space+0x55f/0xaa0 fs/jffs2/nodemgmt.c:197
 do_jffs2_setxattr+0x212/0x1570 fs/jffs2/xattr.c:1117
 __vfs_setxattr+0x118/0x180 fs/xattr.c:182
 __vfs_setxattr_noperm+0x125/0x600 fs/xattr.c:216
 __vfs_setxattr_locked+0x1d3/0x260 fs/xattr.c:277
 vfs_setxattr+0x13f/0x340 fs/xattr.c:309
 setxattr+0x14a/0x160 fs/xattr.c:617
 path_setxattr+0x19b/0x1d0 fs/xattr.c:636
 __do_sys_setxattr fs/xattr.c:652 [inline]
 __se_sys_setxattr fs/xattr.c:648 [inline]
 __x64_sys_setxattr+0xc0/0x160 fs/xattr.c:648
 do_syscall_64+0x35/0x80 arch/x86/entry/common.c:81

The error occurs when trying to create a new attribute of a file by xattr
syscall. Size and name of the attribure are set by user. Current limit of
total size of an attribute is equal to free size in a clean block and it
doesn't include space needed for summary data structures. So it is
possible to create an attribute whose size doesn't exceed the limit but
the total size of attribute and its summary data does. If requested size
of an attribute satisfies this condition, it leads to the following
behavior:

jffs2_do_reserve_space tries to reserve requested size for an attribute
and its summary. It fails to do so because even a clean block doesn't have
enough free space. Then it writes existing summary to the current block
and proceeds to the next block. Summary data is linked to a specific erase
block so it needs to be written to the current block before selecting
a new one.

Then this function is called again to reserve space in a new block.
It fails again and tries to write summary as the first time but at this
point collected summary for the block is empty and it leads to BUG() in
jffs2_summary_write_sumnode.

Decrease maximum allowed size of xattr buffer.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: e631ddba5887 ("[JFFS2] Add erase block summary support (mount time improvement)")
Signed-off-by: Leonid Arapov <arapovl839@gmail.com>
---
 fs/jffs2/xattr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Richard Weinberger May 22, 2025, 7:05 p.m. UTC | #1
----- Ursprüngliche Mail -----
> Von: "Leonid Arapov" <arapovl839@gmail.com>
> An: "David Woodhouse" <dwmw2@infradead.org>
> CC: "Leonid Arapov" <arapovl839@gmail.com>, "richard" <richard@nod.at>, "Ferenc Havasi" <havasi@inf.u-szeged.hu>, "tglx"
> <tglx@linutronix.de>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>
> Gesendet: Mittwoch, 23. April 2025 21:32:05
> Betreff: [PATCH] jffs2: Decrease xattr length limit to avoid summary write error

> When fuzzing, the following error is observed:
> 
> jffs2: warning: (1096) jffs2_sum_write_sumnode: Empty summary info!!!
> ------------[ cut here ]------------
> kernel BUG at fs/jffs2/summary.c:865!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI
> CPU: 1 PID: 1096 Comm: syz-executor340 Not tainted
> 6.1.108-syzkaller-00007-g86fb5a1a71c9 #0
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1
> 04/01/2014
> RIP: 0010:jffs2_sum_write_sumnode.cold+0x195/0x43b fs/jffs2/summary.c:865
> Call Trace:
> jffs2_do_reserve_space+0xa18/0xd60 fs/jffs2/nodemgmt.c:388
> jffs2_reserve_space+0x55f/0xaa0 fs/jffs2/nodemgmt.c:197
> do_jffs2_setxattr+0x212/0x1570 fs/jffs2/xattr.c:1117
> __vfs_setxattr+0x118/0x180 fs/xattr.c:182
> __vfs_setxattr_noperm+0x125/0x600 fs/xattr.c:216
> __vfs_setxattr_locked+0x1d3/0x260 fs/xattr.c:277
> vfs_setxattr+0x13f/0x340 fs/xattr.c:309
> setxattr+0x14a/0x160 fs/xattr.c:617
> path_setxattr+0x19b/0x1d0 fs/xattr.c:636
> __do_sys_setxattr fs/xattr.c:652 [inline]
> __se_sys_setxattr fs/xattr.c:648 [inline]
> __x64_sys_setxattr+0xc0/0x160 fs/xattr.c:648
> do_syscall_64+0x35/0x80 arch/x86/entry/common.c:81
> 
> The error occurs when trying to create a new attribute of a file by xattr
> syscall. Size and name of the attribure are set by user. Current limit of
> total size of an attribute is equal to free size in a clean block and it
> doesn't include space needed for summary data structures. So it is
> possible to create an attribute whose size doesn't exceed the limit but
> the total size of attribute and its summary data does. If requested size
> of an attribute satisfies this condition, it leads to the following
> behavior:
> 
> jffs2_do_reserve_space tries to reserve requested size for an attribute
> and its summary. It fails to do so because even a clean block doesn't have
> enough free space. Then it writes existing summary to the current block
> and proceeds to the next block. Summary data is linked to a specific erase
> block so it needs to be written to the current block before selecting
> a new one.
> 
> Then this function is called again to reserve space in a new block.
> It fails again and tries to write summary as the first time but at this
> point collected summary for the block is empty and it leads to BUG() in
> jffs2_summary_write_sumnode.
> 
> Decrease maximum allowed size of xattr buffer.
> 
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Fixes: e631ddba5887 ("[JFFS2] Add erase block summary support (mount time
> improvement)")
> Signed-off-by: Leonid Arapov <arapovl839@gmail.com>
> ---
> fs/jffs2/xattr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
> index defb4162c3d5..7380f32e6d0f 100644
> --- a/fs/jffs2/xattr.c
> +++ b/fs/jffs2/xattr.c
> @@ -1110,7 +1110,8 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix,
> const char *xname,
> 		return rc;
> 
> 	request = PAD(sizeof(struct jffs2_raw_xattr) + strlen(xname) + 1 + size);
> -	if (request > c->sector_size - c->cleanmarker_size)
> +	if (request > c->sector_size - c->cleanmarker_size -
> +	    JFFS2_SUMMARY_XATTR_SIZE - JFFS2_SUMMARY_FRAME_SIZE)
> 		return -ERANGE;

What about systems without summary support and existing filesystems?
We need to be super careful with such changes.

Thanks,
//richard
diff mbox series

Patch

diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index defb4162c3d5..7380f32e6d0f 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -1110,7 +1110,8 @@  int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
 		return rc;
 
 	request = PAD(sizeof(struct jffs2_raw_xattr) + strlen(xname) + 1 + size);
-	if (request > c->sector_size - c->cleanmarker_size)
+	if (request > c->sector_size - c->cleanmarker_size -
+	    JFFS2_SUMMARY_XATTR_SIZE - JFFS2_SUMMARY_FRAME_SIZE)
 		return -ERANGE;
 
 	rc = jffs2_reserve_space(c, request, &length,