[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <731d5b3e-702d-4f2c-9970-67f1452ccfbb@huawei.com>
Date: Mon, 26 Jan 2026 09:41:47 +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>, <jfs-discussion@...ts.sourceforge.net>,
<linux-kernel@...r.kernel.org>, yangerkun <yangerkun@...wei.com>,
<linan122@...wei.com>
Subject: Re: [PATCH v2] jfs: add dmapctl integrity check to prevent invalid
operations
Ping again...
Thanks
在 2025/12/22 20:05, Li Lingfeng 写道:
> Friendly ping...
>
> Thanks
>
> 在 2025/11/29 9:15, Li Lingfeng 写道:
>> Hi Yun,
>>
>> 在 2025/11/28 23:51, 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..3f1ed82baa3d 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)) {
>>> + jfs_err("dmapctl: invalid nleafs %u (max %u)",
>>> + nleafs, LPERCTL);
>>> + return false;
>>> + }
>>> +
>>> + l2nleafs = le32_to_cpu(dcp->l2nleafs);
>>> + if (unlikely(l2nleafs > L2LPERCTL)) {
>>> + 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 > (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;
>>
>> Thank you for the patch.
>> It also resolves the issue described in the link[1].
>> [1]
>> https://lore.kernel.org/all/c201cbe0-5f52-4572-a04c-dc165eccff95@huawei.com/
>>
>> Tested-by: Li Lingfeng <lilingfeng3@...wei.com>
>>
Powered by blists - more mailing lists