mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dave Kleikamp <dave.kleikamp@oracle.com>
To: Li Lingfeng <lilingfeng3@huawei.com>, Yun Zhou <yun.zhou@windriver.com>
Cc: jfs-discussion@lists.sourceforge.net,
	yangerkun <yangerkun@huawei.com>,
	linux-kernel@vger.kernel.org, linan122@huawei.com,
	kovalev@altlinux.org, contact@arnaud-lcm.com,
	zheng.yu@northwestern.edu, rand.sec96@gmail.com
Subject: Re: [Jfs-discussion] [PATCH v2] jfs: add dmapctl integrity check to prevent invalid operations
Date: Mon, 16 Mar 2026 16:12:14 -0500	[thread overview]
Message-ID: <a86f0b2e-77eb-4fc0-9e0b-07c72492eb6b@oracle.com> (raw)
In-Reply-To: <731d5b3e-702d-4f2c-9970-67f1452ccfbb@huawei.com>

On 1/25/26 7:41PM, Li Lingfeng via Jfs-discussion wrote:
> Ping again...
> 
> Thanks
> 
> 在 2025/12/22 20:05, Li Lingfeng 写道:
>> Friendly ping...

This is finally tested and applied.

Thanks for your patience,

Shaggy

>>
>> 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@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..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@huawei.com>
>>>
> 
> 
> _______________________________________________
> Jfs-discussion mailing list
> Jfs-discussion@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/jfs-discussion


      reply	other threads:[~2026-03-16 21:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-28 15:51 Yun Zhou
2025-11-29  1:15 ` Li Lingfeng
2025-12-22 12:05   ` Li Lingfeng
2026-01-26  1:41     ` Li Lingfeng
2026-03-16 21:12       ` Dave Kleikamp [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a86f0b2e-77eb-4fc0-9e0b-07c72492eb6b@oracle.com \
    --to=dave.kleikamp@oracle.com \
    --cc=contact@arnaud-lcm.com \
    --cc=jfs-discussion@lists.sourceforge.net \
    --cc=kovalev@altlinux.org \
    --cc=lilingfeng3@huawei.com \
    --cc=linan122@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rand.sec96@gmail.com \
    --cc=yangerkun@huawei.com \
    --cc=yun.zhou@windriver.com \
    --cc=zheng.yu@northwestern.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®