| 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
| ||
|
Message-ID: <c201cbe0-5f52-4572-a04c-dc165eccff95@huawei.com>
Date: Mon, 24 Nov 2025 19:42:52 +0800
From: Li Lingfeng <lilingfeng3@...wei.com>
To: Yun Zhou <yun.zhou@...driver.com>, <shaggy@...nel.org>
CC: <rand.sec96@...il.com>, <contact@...aud-lcm.com>, <kovalev@...linux.org>,
<zheng.yu@...thwestern.edu>, <eadavis@...com>,
<jfs-discussion@...ts.sourceforge.net>, <linux-kernel@...r.kernel.org>,
<linan122@...wei.com>, yangerkun <yangerkun@...wei.com>
Subject: Re: [PATCH] jfs: add dmapctl integrity check to prevent invalid
operations
Hi Yun,
Recently, we triggered a UBSAN warning through syzkaller:
[ 126.922474][ T769] UBSAN: shift-out-of-bounds in
fs/jfs/jfs_dmap.c:2646:11
[ 126.923505][ T769] shift exponent 110 is too large for 32-bit type 'int'
[ 126.924543][ T769] CPU: 14 UID: 0 PID: 769 Comm: repro Not tainted
6.18.0-rc6+ #127 PREEMPT(none)
[ 126.924549][ T769] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.16.3-2.fc40 04/01/2014
[ 126.924552][ T769] Call Trace:
[ 126.924555][ T769] <TASK>
[ 126.924557][ T769] dump_stack_lvl+0x4b/0x70
[ 126.924572][ T769] ubsan_epilogue+0x5/0x2b
[ 126.924583][ T769] __ubsan_handle_shift_out_of_bounds.cold+0x61/0xe6
[ 126.924588][ T769] ? do_read_cache_folio+0x9c/0x330
[ 126.924598][ T769] dbSplit+0x153/0x190
[ 126.924607][ T769] dbAdjCtl+0x413/0x6b1
[ 126.924613][ T769] dbAllocDmap+0xbc/0xe4
[ 126.924618][ T769] dbAlloc+0x5df/0x803
[ 126.924624][ T769] ea_write+0x26f/0x628
[ 126.924629][ T769] ? ea_get+0x639/0x1260
[ 126.924634][ T769] ? __pfx_ea_write+0x10/0x10
[ 126.924637][ T769] ? __pfx__printk+0x10/0x10
[ 126.924645][ T769] ? __pfx_ea_get+0x10/0x10
[ 126.924649][ T769] ea_put+0x1b5/0x567
[ 126.924653][ T769] __jfs_setxattr.cold+0x4e8/0x632
[ 126.924658][ T769] ? __pfx___jfs_setxattr+0x10/0x10
[ 126.924661][ T769] ? __pfx__printk+0x10/0x10
[ 126.924665][ T769] ? mutex_lock+0x86/0xe0
[ 126.924675][ T769] ? __pfx_mutex_lock+0x10/0x10
[ 126.924681][ T769] __jfs_xattr_set+0xe4/0x149
[ 126.924685][ T769] ? __pfx___jfs_xattr_set+0x10/0x10
[ 126.924689][ T769] ? xattr_full_name+0x3a/0x80
[ 126.924693][ T769] __vfs_setxattr+0x118/0x150
[ 126.924699][ T769] ? __pfx___vfs_setxattr+0x10/0x10
[ 126.924703][ T769] ? security_inode_setxattr+0x1a2/0x2a0
[ 126.924711][ T769] __vfs_setxattr_noperm.cold+0x1f/0x59
[ 126.924716][ T769] vfs_setxattr+0x11b/0x300
[ 126.924720][ T769] ? __pfx_vfs_setxattr+0x10/0x10
[ 126.924724][ T769] ? check_heap_object+0x6f/0x430
[ 126.924731][ T769] ? do_setxattr+0xa7/0x150
[ 126.924734][ T769] filename_setxattr+0x124/0x160
[ 126.924738][ T769] ? __pfx_filename_setxattr+0x10/0x10
[ 126.924742][ T769] ? getname_flags.part.0+0xf8/0x480
[ 126.924749][ T769] path_setxattrat+0x215/0x290
[ 126.924753][ T769] ? __pfx_path_setxattrat+0x10/0x10
[ 126.924757][ T769] ? __call_rcu_common.constprop.0+0x341/0x970
[ 126.924767][ T769] ? __pfx___call_rcu_common.constprop.0+0x10/0x10
[ 126.924772][ T769] ? kmem_cache_free+0x3dd/0x5d0
[ 126.924778][ T769] ? kmem_cache_free+0x40b/0x5d0
[ 126.924781][ T769] ? fput_close_sync+0xdc/0x190
[ 126.924789][ T769] ? fput_close_sync+0xdc/0x190
[ 126.924792][ T769] ? __pfx_fput_close_sync+0x10/0x10
[ 126.924796][ T769] ? file_close_fd_locked+0x178/0x2a0
[ 126.924803][ T769] __x64_sys_lsetxattr+0xc9/0x140
[ 126.924807][ T769] do_syscall_64+0x61/0x9d0
[ 126.924814][ T769] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 126.924818][ T769] RIP: 0033:0x44c84d
[ 126.924823][ T769] Code: 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 f3
0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c
24 08 0f 05 <48> 3d 01 f0 ff ff 738
[ 126.924827][ T769] RSP: 002b:00007ffcbf892088 EFLAGS: 00000287
ORIG_RAX: 00000000000000bd
[ 126.924833][ T769] RAX: ffffffffffffffda RBX: 00007ffcbf892278 RCX:
000000000044c84d
[ 126.924835][ T769] RDX: 0000000000000000 RSI: 0000200000000200 RDI:
0000200000000040
[ 126.924838][ T769] RBP: 00007ffcbf892090 R08: 0000000000000000 R09:
0000000000000001
[ 126.924840][ T769] R10: 0000000000000000 R11: 0000000000000287 R12:
0000000000000001
[ 126.924842][ T769] R13: 00007ffcbf892268 R14: 00000000004c38d0 R15:
0000000000000001
[ 126.924848][ T769] </TASK>
[ 126.924850][ T769] ---[ end trace ]---
The warning occurred because syzkaller constructed a malformed image, and
JFS read an invalid leaf value from it.
In our testing, this patch resolves the issue by preventing the use of the
invalid value:
[ 39.890789][ T765] dmapctl: leaf value 124 too large at index 341
[ 39.891684][ T765] ERROR: (device loop0): dbAdjCtl: Corrupt dmapctl page
[ 39.891684][ T765]
[ 39.893343][ T765] ERROR: (device loop0): remounting filesystem as
read-only
However, I noticed that this patch triggers some build warnings.
Could you please help address these warnings and push the fix upstream?
Thanks,
Lingfeng.
在 2025/11/20 23:44, Yun Zhou 写道:
> Add check_dmapctl() to validate dmapctl structure integrity, focusing on
> preventing invalid operations caused by on-disk corruption.
>
> Key checks:
> - nleafs bounded by [0, LPERCTL] (maximum leaf nodes per dmapctl).
> - l2nleafs bounded by [0, L2LPERCTL] and consistent with nleafs
> (nleafs must be 2^l2nleafs).
> - leafidx must be exactly CTLLEAFIND (expected leaf index position).
> - height bounded by [0, L2LPERCTL >> 1] (valid tree height range).
> - budmin validity: NOFREE only if nleafs=0; otherwise >= BUDMIN.
> - Leaf nodes fit within stree array (leafidx + nleafs <= CTLTREESIZE).
> - Leaf node values are either non-negative or NOFREE.
>
> Invoked in dbAllocAG(), dbFindCtl(), dbAdjCtl() and dbExtendFS() when
> accessing dmapctl pages, catching corruption early before dmap operations
> trigger invalid memory access or logic errors.
>
> This fixes the following UBSAN warning.
>
> [58245.668090][T14017] ------------[ cut here ]------------
> [58245.668103][T14017] UBSAN: shift-out-of-bounds in fs/jfs/jfs_dmap.c:2641:11
> [58245.668119][T14017] shift exponent 110 is too large for 32-bit type 'int'
> [58245.668137][T14017] CPU: 0 UID: 0 PID: 14017 Comm: 4c1966e88c28fa9 Tainted: G E 6.18.0-rc4-00253-g21ce5d4ba045-dirty #124 PREEMPT_{RT,(full)}
> [58245.668174][T14017] Tainted: [E]=UNSIGNED_MODULE
> [58245.668176][T14017] Hardware name: QEMU Ubuntu 25.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [58245.668184][T14017] Call Trace:
> [58245.668200][T14017] <TASK>
> [58245.668208][T14017] dump_stack_lvl+0x189/0x250
> [58245.668288][T14017] ? __pfx_dump_stack_lvl+0x10/0x10
> [58245.668301][T14017] ? __pfx__printk+0x10/0x10
> [58245.668315][T14017] ? lock_metapage+0x303/0x400 [jfs]
> [58245.668406][T14017] ubsan_epilogue+0xa/0x40
> [58245.668422][T14017] __ubsan_handle_shift_out_of_bounds+0x386/0x410
> [58245.668462][T14017] dbSplit+0x1f8/0x200 [jfs]
> [58245.668543][T14017] dbAdjCtl+0x34c/0xa20 [jfs]
> [58245.668628][T14017] dbAllocNear+0x2ee/0x3d0 [jfs]
> [58245.668710][T14017] dbAlloc+0x933/0xba0 [jfs]
> [58245.668797][T14017] ea_write+0x374/0xdd0 [jfs]
> [58245.668888][T14017] ? __pfx_ea_write+0x10/0x10 [jfs]
> [58245.668966][T14017] ? __jfs_setxattr+0x76e/0x1120 [jfs]
> [58245.669046][T14017] __jfs_setxattr+0xa01/0x1120 [jfs]
> [58245.669135][T14017] ? __pfx___jfs_setxattr+0x10/0x10 [jfs]
> [58245.669216][T14017] ? mutex_lock_nested+0x154/0x1d0
> [58245.669252][T14017] ? __jfs_xattr_set+0xb9/0x170 [jfs]
> [58245.669333][T14017] __jfs_xattr_set+0xda/0x170 [jfs]
> [58245.669430][T14017] ? __pfx___jfs_xattr_set+0x10/0x10 [jfs]
> [58245.669509][T14017] ? xattr_full_name+0x6f/0x90
> [58245.669546][T14017] ? jfs_xattr_set+0x33/0x60 [jfs]
> [58245.669636][T14017] ? __pfx_jfs_xattr_set+0x10/0x10 [jfs]
> [58245.669726][T14017] __vfs_setxattr+0x43c/0x480
> [58245.669743][T14017] __vfs_setxattr_noperm+0x12d/0x660
> [58245.669756][T14017] vfs_setxattr+0x16b/0x2f0
> [58245.669768][T14017] ? __pfx_vfs_setxattr+0x10/0x10
> [58245.669782][T14017] filename_setxattr+0x274/0x600
> [58245.669795][T14017] ? __pfx_filename_setxattr+0x10/0x10
> [58245.669806][T14017] ? getname_flags+0x1e5/0x540
> [58245.669829][T14017] path_setxattrat+0x364/0x3a0
> [58245.669840][T14017] ? __pfx_path_setxattrat+0x10/0x10
> [58245.669859][T14017] ? __se_sys_chdir+0x1b9/0x280
> [58245.669876][T14017] __x64_sys_lsetxattr+0xbf/0xe0
> [58245.669888][T14017] do_syscall_64+0xfa/0xfa0
> [58245.669901][T14017] ? lockdep_hardirqs_on+0x9c/0x150
> [58245.669913][T14017] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [58245.669927][T14017] ? exc_page_fault+0xab/0x100
> [58245.669937][T14017] entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Reported-by: syzbot+4c1966e88c28fa96e053@...kaller.appspotmail.com
> Signed-off-by: Yun Zhou <yun.zhou@...driver.com>
> ---
> fs/jfs/jfs_dmap.c | 113 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 110 insertions(+), 3 deletions(-)
>
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index cdfa699cd7c8..20dffd44785f 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -133,6 +133,92 @@ static const s8 budtab[256] = {
> 2, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, -1
> };
>
> +/*
> + * check_dmapctl - Validate integrity of a dmapctl structure
> + * @dcp: Pointer to the dmapctl structure to check
> + *
> + * Return: true if valid, false if corrupted
> + */
> +static bool check_dmapctl(struct dmapctl *dcp)
> +{
> + s8 budmin = dcp->budmin;
> + u32 nleafs, l2nleafs, leafidx, height;
> + int i;
> +
> + nleafs = le32_to_cpu(dcp->nleafs);
> + /* Check basic field ranges */
> + if (unlikely(nleafs > LPERCTL || nleafs < 0)) {
> + jfs_err("dmapctl: invalid nleafs %u (max %u)",
> + nleafs, LPERCTL);
> + return false;
> + }
> +
> + l2nleafs = le32_to_cpu(dcp->l2nleafs);
> + if (unlikely(l2nleafs > L2LPERCTL || l2nleafs < 0)) {
> + jfs_err("dmapctl: invalid l2nleafs %u (max %u)",
> + l2nleafs, L2LPERCTL);
> + return false;
> + }
> +
> + /* Verify nleafs matches l2nleafs (must be power of two) */
> + if (unlikely((1U << l2nleafs) != nleafs)) {
> + jfs_err("dmapctl: nleafs %u != 2^%u",
> + nleafs, l2nleafs);
> + return false;
> + }
> +
> + leafidx = le32_to_cpu(dcp->leafidx);
> + /* Check leaf index matches expected position */
> + if (unlikely(leafidx != CTLLEAFIND)) {
> + jfs_err("dmapctl: invalid leafidx %u (expected %u)",
> + leafidx, CTLLEAFIND);
> + return false;
> + }
> +
> + height = le32_to_cpu(dcp->height);
> + /* Check tree height is within valid range */
> + if (unlikely(height < 0 || height > (L2LPERCTL >> 1))) {
> + jfs_err("dmapctl: invalid height %u (max %u)",
> + height, L2LPERCTL >> 1);
> + return false;
> + }
> +
> + /* Check budmin is valid (cannot be NOFREE for non-empty tree) */
> + if (budmin == NOFREE) {
> + if (unlikely(nleafs > 0)) {
> + jfs_err("dmapctl: budmin is NOFREE but nleafs %u",
> + nleafs);
> + return false;
> + }
> + } else if (unlikely(budmin < BUDMIN)) {
> + jfs_err("dmapctl: invalid budmin %d (min %d)",
> + budmin, BUDMIN);
> + return false;
> + }
> +
> + /* Check leaf nodes fit within stree array */
> + if (unlikely(leafidx + nleafs > CTLTREESIZE)) {
> + jfs_err("dmapctl: leaf range exceeds stree size (end %u > %u)",
> + leafidx + nleafs, CTLTREESIZE);
> + return false;
> + }
> +
> + /* Check leaf nodes have valid values */
> + for (i = leafidx; i < leafidx + nleafs; i++) {
> + s8 val = dcp->stree[i];
> + if (unlikely(val < NOFREE)) {
> + jfs_err("dmapctl: invalid leaf value %d at index %d",
> + val, i);
> + return false;
> + } else if (unlikely(val > 31)) {
> + jfs_err("dmapctl: leaf value %d too large at index %d", val, i);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> /*
> * NAME: dbMount()
> *
> @@ -1372,7 +1458,7 @@ dbAllocAG(struct bmap * bmp, int agno, s64 nblocks, int l2nb, s64 * results)
> dcp = (struct dmapctl *) mp->data;
> budmin = dcp->budmin;
>
> - if (dcp->leafidx != cpu_to_le32(CTLLEAFIND)) {
> + if (unlikely(!check_dmapctl(dcp))) {
> jfs_error(bmp->db_ipbmap->i_sb, "Corrupt dmapctl page\n");
> release_metapage(mp);
> return -EIO;
> @@ -1702,7 +1788,7 @@ static int dbFindCtl(struct bmap * bmp, int l2nb, int level, s64 * blkno)
> dcp = (struct dmapctl *) mp->data;
> budmin = dcp->budmin;
>
> - if (dcp->leafidx != cpu_to_le32(CTLLEAFIND)) {
> + if (unlikely(!check_dmapctl(dcp))) {
> jfs_error(bmp->db_ipbmap->i_sb,
> "Corrupt dmapctl page\n");
> release_metapage(mp);
> @@ -2485,7 +2571,7 @@ dbAdjCtl(struct bmap * bmp, s64 blkno, int newval, int alloc, int level)
> return -EIO;
> dcp = (struct dmapctl *) mp->data;
>
> - if (dcp->leafidx != cpu_to_le32(CTLLEAFIND)) {
> + if (unlikely(!check_dmapctl(dcp))) {
> jfs_error(bmp->db_ipbmap->i_sb, "Corrupt dmapctl page\n");
> release_metapage(mp);
> return -EIO;
> @@ -3454,6 +3540,11 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)
> return -EIO;
> }
> l2dcp = (struct dmapctl *) l2mp->data;
> + if (unlikely(!check_dmapctl(l2dcp))) {
> + jfs_error(ipbmap->i_sb, "Corrupt dmapctl page\n");
> + release_metapage(l2mp);
> + return -EIO;
> + }
>
> /* compute start L1 */
> k = blkno >> L2MAXL1SIZE;
> @@ -3471,6 +3562,10 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)
> if (l1mp == NULL)
> goto errout;
> l1dcp = (struct dmapctl *) l1mp->data;
> + if (unlikely(!check_dmapctl(l1dcp))) {
> + jfs_error(ipbmap->i_sb, "Corrupt dmapctl page\n");
> + goto errout;
> + }
>
> /* compute start L0 */
> j = (blkno & (MAXL1SIZE - 1)) >> L2MAXL0SIZE;
> @@ -3484,6 +3579,10 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)
> goto errout;
>
> l1dcp = (struct dmapctl *) l1mp->data;
> + if (unlikely(!check_dmapctl(l1dcp))) {
> + jfs_error(ipbmap->i_sb, "Corrupt dmapctl page\n");
> + goto errout;
> + }
>
> /* compute start L0 */
> j = 0;
> @@ -3503,6 +3602,10 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)
> if (l0mp == NULL)
> goto errout;
> l0dcp = (struct dmapctl *) l0mp->data;
> + if (unlikely(!check_dmapctl(l0dcp))) {
> + jfs_error(ipbmap->i_sb, "Corrupt dmapctl page\n");
> + goto errout;
> + }
>
> /* compute start dmap */
> i = (blkno & (MAXL0SIZE - 1)) >>
> @@ -3518,6 +3621,10 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno, s64 nblocks)
> goto errout;
>
> l0dcp = (struct dmapctl *) l0mp->data;
> + if (unlikely(!check_dmapctl(l0dcp))) {
> + jfs_error(ipbmap->i_sb, "Corrupt dmapctl page\n");
> + goto errout;
> + }
>
> /* compute start dmap */
> i = 0;
Powered by blists - more mailing lists