1. Patchew
  2. linux
  3. View series

[PATCH] jfs: add dmapctl integrity check to prevent invalid operations

Yun Zhou posted 1 patch 1 week, 4 days ago
There is a newer version of this series
fs/jfs/jfs_dmap.c | 113 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 110 insertions(+), 3 deletions(-)
[PATCH] jfs: add dmapctl integrity check to prevent invalid operations
Posted by Yun Zhou 1 week, 4 days ago 
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@syzkaller.appspotmail.com
Signed-off-by: Yun Zhou <yun.zhou@windriver.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;
-- 
2.34.1
Re: [PATCH] jfs: add dmapctl integrity check to prevent invalid operations
Posted by Li Lingfeng 1 week ago 
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@syzkaller.appspotmail.com
> Signed-off-by: Yun Zhou <yun.zhou@windriver.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;
Re: [PATCH] jfs: add dmapctl integrity check to prevent invalid operations
Posted by Zhou, Yun 4 days ago 
Hi Lingfeng,



On 11/24/25 19:42, Li Lingfeng wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender 
> and know the content is safe.
>
> 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? 
I wonder what build warnings you encountered, since I have not seen it.

Thanks,
Yun
Re: [PATCH] jfs: add dmapctl integrity check to prevent invalid operations
Posted by Li Lingfeng 4 days ago 
Hi Yun,

在 2025/11/28 8:31, Zhou, Yun 写道:
> Hi Lingfeng,
>
>
>
> On 11/24/25 19:42, Li Lingfeng wrote:
>> CAUTION: This email comes from a non Wind River email account!
>> Do not click links or open attachments unless you recognize the 
>> sender and know the content is safe.
>>
>> 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? 
> I wonder what build warnings you encountered, since I have not seen it.

Here is the build warning noticed by kernel test robot:
https://lore.kernel.org/all/202511211750.bjcw3Ucd-lkp@intel.com/

These build warnings appear to be caused by comparisons between unsigned
values and zero. I'm not familiar with JFS, so I'm not sure whether this
logic is necessary or if there is an alternative way to handle the checks.
Could you take a look?

Thanks,
Lingfeng.

>
> Thanks,
> Yun

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.