Re: [PATCH RFC v3] jfs: validate BT_ROOT flag in xt_getpage and DT_GETPAGE
Kusaram Devineni <[email protected]> Wed, 29 Jul 2026 14:38:02 +0530
| Newsgroups | dev.linux.lists.syzbot |
|---|---|
| Message-ID | <[email protected]> |
On 15-07-2026 08:58 pm, syzbot wrote:
> A corrupted JFS image can trick the kernel into calling release_metapage()
> on a pseudo-metapage embedded within the inode, leading to an out-of-bounds
> memory access and a spinlock crash.
>
> When operating on the root page of an xtree or dtree (which is stored
> inline within the inode), JFS doesn't use a real struct metapage. Instead,
> BT_GETPAGE() sets the metapage pointer to the address of
> JFS_IP(ip)->bxflag. Because this is a pseudo-metapage, it does not have a
> valid wait_queue_head_t wait field. Accessing mp->wait reads out-of-bounds
> memory in struct jfs_inode_info.
>
> In xt_getpage(), the code validates nextindex and maxentry, but it fails to
> verify that the BT_ROOT flag is set in p->header.flag for the root page (bn
> == 0). If the inline xtree root page has a corrupted header.flag where
> BT_ROOT is missing, xtTruncate() will mistakenly treat the root page as a
> regular leaf page and call discard_metapage(mp).
>
> Unlike XT_PUTPAGE() which safely checks !BT_IS_ROOT(mp) before releasing,
> discard_metapage() calls release_metapage(mp) directly. This assumes mp is
> a real metapage and calls unlock_metapage(mp), which executes
> wake_up(&mp->wait), accessing garbage memory and resulting in a spinlock
> bad magic BUG or UBSAN out-of-bounds array access.
>
> Fix this by enforcing the invariant that the BT_ROOT flag is set if and
> only if bn == 0 in both xt_getpage() and DT_GETPAGE().
>
> BUG: spinlock bad magic on CPU#1, syz-executor/6078
> lock: 0xffff8881f55f1b48, .magic: ffffffff, .owner: /-1902340672,
> .owner_cpu: 768
> CPU: 1 UID: 0 PID: 6078 Comm: syz-executor Not tainted syzkaller #1
> PREEMPT(full)
> Call Trace:
> <TASK>
> dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
> spin_bug kernel/locking/spinlock_debug.c:78 [inline]
> debug_spin_lock_before kernel/locking/spinlock_debug.c:86 [inline]
> do_raw_spin_lock+0x1e5/0x2f0 kernel/locking/spinlock_debug.c:115
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:133 [inline]
> _raw_spin_lock_irqsave+0x4c/0x60 kernel/locking/spinlock.c:166
> __wake_up_common_lock+0x30/0x1f0 kernel/sched/wait.c:124
> unlock_metapage fs/jfs/jfs_metapage.c:40 [inline]
> release_metapage+0x131/0xa60 fs/jfs/jfs_metapage.c:872
> xtTruncate+0xeaa/0x2eb0 fs/jfs/jfs_xtree.c:-1
> jfs_free_zero_link+0x35b/0x4c0 fs/jfs/namei.c:760
> jfs_evict_inode+0x356/0x430 fs/jfs/inode.c:159
> evict+0x624/0xb50 fs/inode.c:841
> ...
> </TASK>
>
> UBSAN: array-index-out-of-bounds in kernel/locking/qspinlock.h:68:9
> index 8945 is out of range for type 'unsigned long[8]'
> CPU: 1 UID: 0 PID: 6078 Comm: syz-executor Not tainted syzkaller #1
> PREEMPT(full)
> Call Trace:
> <TASK>
> dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
> ubsan_epilogue+0xa/0x30 lib/ubsan.c:233
> __ubsan_handle_out_of_bounds+0xe8/0xf0 lib/ubsan.c:455
> decode_tail kernel/locking/qspinlock.h:68 [inline]
> __pv_queued_spin_lock_slowpath+0xaf3/0xbc0 kernel/locking/qspinlock.c:285
> pv_queued_spin_lock_slowpath arch/x86/include/asm/paravirt-spinlock.h:35
> [inline]
> queued_spin_lock_slowpath arch/x86/include/asm/paravirt-spinlock.h:66
> [inline]
> queued_spin_lock include/asm-generic/qspinlock.h:114 [inline]
> do_raw_spin_lock+0x26c/0x2f0 kernel/locking/spinlock_debug.c:116
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:133 [inline]
> _raw_spin_lock_irqsave+0x4c/0x60 kernel/locking/spinlock.c:166
> __wake_up_common_lock+0x30/0x1f0 kernel/sched/wait.c:124
> unlock_metapage fs/jfs/jfs_metapage.c:40 [inline]
> release_metapage+0x131/0xa60 fs/jfs/jfs_metapage.c:872
> xtTruncate+0xeaa/0x2eb0 fs/jfs/jfs_xtree.c:-1
> ...
> </TASK>
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview syzbot
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=e380443eaa59bfb75a84
> Link: https://syzkaller.appspot.com/ai_job?id=20a14f45-b85b-4c13-8896-26c43b87a29b
> To: <[email protected]>
> To: "Dave Kleikamp" <[email protected]>
> Cc: "Arnd Bergmann" <[email protected]>
> Cc: <[email protected]>
> Cc: "Yun Zhou" <[email protected]>
> Cc: "Zheng Yu" <[email protected]>
>
> ---
> v3:
> - Validated root/non-root invariant failures and non-root dtpage integrity failures.
update the above changelog statement with something simpler, like below...
"Kept the existing DT_GETPAGE error message for both corruption checks."
> - Removed '------------[ cut here ]------------' from the commit message.
>
> v2:
> - Combined the BT_ROOT flag validation with the existing page integrity check in DT_GETPAGE.
> - Unified the error message in DT_GETPAGE for both root and leaf page corruption.
> https://lore.kernel.org/all/[email protected]/T/
>
> v1:
> https://lore.kernel.org/all/[email protected]/T/
> ---
> diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
> index ac0f79faf..2b6bdea1f 100644
> --- a/fs/jfs/jfs_dtree.c
> +++ b/fs/jfs/jfs_dtree.c
> @@ -111,19 +111,20 @@ struct dtsplit {
> #define DT_PAGE(IP, MP) BT_PAGE(IP, MP, dtpage_t, i_dtroot)
>
> /* get page buffer for specified block address */
> -#define DT_GETPAGE(IP, BN, MP, SIZE, P, RC) \
> -do { \
> - BT_GETPAGE(IP, BN, MP, dtpage_t, SIZE, P, RC, i_dtroot); \
> - if (!(RC)) { \
> - if ((BN) && !check_dtpage(P)) { \
> - BT_PUTPAGE(MP); \
> - jfs_error((IP)->i_sb, \
> - "DT_GETPAGE: dtree page corrupt\n"); \
> - MP = NULL; \
> - RC = -EIO; \
> - } \
> - } \
> -} while (0)
> +#define DT_GETPAGE(IP, BN, MP, SIZE, P, RC) \
> + do { \
> + BT_GETPAGE(IP, BN, MP, dtpage_t, SIZE, P, RC, i_dtroot); \
> + if (!(RC)) { \
> + if ((((BN) == 0) != !!((P)->header.flag & BT_ROOT)) || \
> + ((BN) && !check_dtpage(P))) { \
> + BT_PUTPAGE(MP); \
> + jfs_error((IP)->i_sb, \
> + "DT_GETPAGE: dtree page corrupt\n"); \
> + MP = NULL; \
> + RC = -EIO; \
> + } \
> + } \
> + } while (0)
>
in DT_GETPAGE() macro above, preserve the existing tab-aligned macro
style and avoid reindenting the whole macro. limit the change only to
the condition/branch needed for the BT_ROOT invariant check.
> /* for consistency */
> #define DT_PUTPAGE(MP) BT_PUTPAGE(MP)
> diff --git a/fs/jfs/jfs_xtree.c b/fs/jfs/jfs_xtree.c
> index 28c3cf960..6d960dd96 100644
> --- a/fs/jfs/jfs_xtree.c
> +++ b/fs/jfs/jfs_xtree.c
> @@ -118,10 +118,11 @@ static inline xtpage_t *xt_getpage(struct inode *ip, s64 bn, struct metapage **m
> if (rc)
> return ERR_PTR(rc);
> if ((le16_to_cpu(p->header.nextindex) < XTENTRYSTART) ||
> - (le16_to_cpu(p->header.nextindex) >
> - le16_to_cpu(p->header.maxentry)) ||
> - (le16_to_cpu(p->header.maxentry) >
> - ((bn == 0) ? XTROOTMAXSLOT : PSIZE >> L2XTSLOTSIZE))) {
> + (le16_to_cpu(p->header.nextindex) >
> + le16_to_cpu(p->header.maxentry)) ||
> + (le16_to_cpu(p->header.maxentry) >
> + ((bn == 0) ? XTROOTMAXSLOT : PSIZE >> L2XTSLOTSIZE)) ||
> + ((bn == 0) != !!(p->header.flag & BT_ROOT))) {
in the change to xt_getpage() block made above, preserve the existing
continuation indentation of the nextindex and maxentry checks. add only
the new BT_ROOT invariant condition.
> jfs_error(ip->i_sb, "xt_getpage: xtree page corrupt\n");
> BT_PUTPAGE(*mp);
> *mp = NULL;
>
>
> base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
-kusaram