| 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: <htz6wcj3y3dhp7jyzgrachhrz6uzi7ytuby6fdtrugm57cik32@2vo56g6wmidb>
Date: Sun, 14 Dec 2025 23:14:46 +0800
From: Heming Zhao <heming.zhao@...e.com>
To: Prithvi <activprithvi@...il.com>, joseph.qi@...ux.alibaba.com
Cc: mark@...heh.com, jlbec@...lplan.org, ocfs2-devel@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org,
david.hunter.linux@...il.com, khalid@...nel.org,
syzbot+c818e5c4559444f88aa0@...kaller.appspotmail.com, stable@...r.kernel.org
Subject: Re: [PATCH v2] ocfs2: fix kernel BUG in ocfs2_write_block
On Sat, Dec 13, 2025 at 02:25:29PM +0530, Prithvi wrote:
> On Thu, Dec 11, 2025 at 10:17:21AM +0800, Joseph Qi wrote:
> >
> >
> > On 2025/12/11 03:32, Prithvi Tambewagh wrote:
> > > When the filesystem is being mounted, the kernel panics while the data
> > > regarding slot map allocation to the local node, is being written to the
> > > disk. This occurs because the value of slot map buffer head block
> > > number, which should have been greater than or equal to
> > > `OCFS2_SUPER_BLOCK_BLKNO` (evaluating to 2) is less than it, indicative
> > > of disk metadata corruption. This triggers
> > > BUG_ON(bh->b_blocknr < OCFS2_SUPER_BLOCK_BLKNO) in ocfs2_write_block(),
> > > causing the kernel to panic.
> > >
> > > This is fixed by introducing an if condition block in
> > > ocfs2_update_disk_slot(), right before calling ocfs2_write_block(), which
> > > checks if `bh->b_blocknr` is lesser than `OCFS2_SUPER_BLOCK_BLKNO`; if
> > > yes, then ocfs2_error is called, which prints the error log, for
> > > debugging purposes, and the return value of ocfs2_error() is returned
> > > back to caller of ocfs2_update_disk_slot() i.e. ocfs2_find_slot(). If
> > > the return value is zero. then error code EIO is returned.
> > >
> > > Reported-by: syzbot+c818e5c4559444f88aa0@...kaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=c818e5c4559444f88aa0
> > > Tested-by: syzbot+c818e5c4559444f88aa0@...kaller.appspotmail.com
> > > Cc: stable@...r.kernel.org
> > > Signed-off-by: Prithvi Tambewagh <activprithvi@...il.com>
> > > ---
> > > v1->v2:
> > > - Remove usage of le16_to_cpu() from ocfs2_error()
> > > - Cast bh->b_blocknr to unsigned long long
> > > - Remove type casting for OCFS2_SUPER_BLOCK_BLKNO
> > > - Fix Sparse warnings reported in v1 by kernel test robot
> > > - Update title from 'ocfs2: Fix kernel BUG in ocfs2_write_block' to
> > > 'ocfs2: fix kernel BUG in ocfs2_write_block'
> > >
> > > v1 link: https://lore.kernel.org/all/20251206154819.175479-1-activprithvi@gmail.com/T/
> > >
> > > fs/ocfs2/slot_map.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/fs/ocfs2/slot_map.c b/fs/ocfs2/slot_map.c
> > > index e544c704b583..e916a2e8f92d 100644
> > > --- a/fs/ocfs2/slot_map.c
> > > +++ b/fs/ocfs2/slot_map.c
> > > @@ -193,6 +193,16 @@ static int ocfs2_update_disk_slot(struct ocfs2_super *osb,
> > > else
> > > ocfs2_update_disk_slot_old(si, slot_num, &bh);
> > > spin_unlock(&osb->osb_lock);
> > > + if (bh->b_blocknr < OCFS2_SUPER_BLOCK_BLKNO) {
> > > + status = ocfs2_error(osb->sb,
> > > + "Invalid Slot Map Buffer Head "
> > > + "Block Number : %llu, Should be >= %d",
> > > + (unsigned long long)bh->b_blocknr,
> > > + OCFS2_SUPER_BLOCK_BLKNO);
> > > + if (!status)
> > > + return -EIO;
> > > + return status;
> > > + }
> > >
> > > status = ocfs2_write_block(osb, bh, INODE_CACHE(si->si_inode));
> > > if (status < 0)
> > >
> >
> > Ummm... The 'bh' is from ocfs2_slot_info, which is load from crafted
> > image during mount.
> > So IIUC, the root cause is we read slot info without validating, see
> > ocfs2_refresh_slot_info().
> > So I'd prefer to implement a validate func and pass it into
> > ocfs2_read_blocks() to do this job.
> >
> > Thanks,
> > Joseph
>
> I checked using GDB and learnt that ocfs2_refresh_slot() is not called
> until the bug is triggered. Checking further, I obtained this backtrace
> just before the bug was triggered, with a breakpoint set at
> ocfs2_read_blocks():
>
> #0 ocfs2_read_blocks (ci=0xffff88804b6e3cc0, block=0, nr=1, bhs=0xffffc9000f37efe0,
> flags=1, validate=0x0) at fs/ocfs2/buffer_head_io.c:197
> #1 0xffffffff83a1fb44 in ocfs2_map_slot_buffers (osb=0xffff888028d2c000,
> si=0xffff888027329d00) at fs/ocfs2/slot_map.c:385
> #2 ocfs2_init_slot_info (osb=0xffff888028d2c000) at fs/ocfs2/slot_map.c:424
> #3 0xffffffff83a91b93 in ocfs2_initialize_super (sb=0xffff88802912a000, bh=<optimized out>,
> sector_size=<optimized out>, stats=<optimized out>) at fs/ocfs2/super.c:2220
> #4 ocfs2_fill_super (sb=0xffff88802912a000, fc=<optimized out>) at fs/ocfs2/super.c:993
> #5 0xffffffff822db16e in get_tree_bdev_flags (fc=0xffff888029dac800,
> fill_super=0xffffffff83a8d5c0 <ocfs2_fill_super>, flags=<optimized out>)
> at fs/super.c:1691
> #6 0xffffffff822db3b2 in vfs_get_tree (fc=0xffff888029dac800) at fs/super.c:1751
> #7 0xffffffff82366f32 in fc_mount (fc=0xffff888029dac800) at fs/namespace.c:1208
> #8 do_new_mount_fc (fc=0xffff888029dac800, mountpoint=0xffffc9000f37fe60, mnt_flags=48)
> at fs/namespace.c:3651
> #9 do_new_mount (path=0xffffc9000f37fe60, fstype=<optimized out>, sb_flags=<optimized out>,
> mnt_flags=48, name=0xffff888024bf93a0 "/dev/loop0", data=0xffff888028b54000)
> at fs/namespace.c:3727
> #10 0xffffffff823659c7 in path_mount (
> dev_name=0x1 <error: Cannot access memory at address 0x1>, path=0xffff888024bf93a0,
> type_page=0xffff888028b54000 "user_xattr", flags=<optimized out>,
> data_page=0x7fffc7d5cb00) at fs/namespace.c:4037
> #11 0xffffffff82369253 in do_mount (dev_name=0xffff888024bf93a0 "/dev/loop0",
> dir_name=0x200000000040 "./file1", type_page=<optimized out>, flags=2240,
> --Type <RET> for more, q to quit, c to continue without paging--
> data_page=0xffff888028b54000) at fs/namespace.c:4050
> #12 __do_sys_mount (type=<optimized out>, dev_name=<optimized out>, dir_name=<optimized out>, flags=<optimized out>,
> data=<optimized out>) at fs/namespace.c:4238
> #13 __se_sys_mount (dev_name=<optimized out>, dir_name=35184372088896, type=<optimized out>, flags=2240,
> data=140736546065152) at fs/namespace.c:4215
> #14 0xffffffff82368f2c in __x64_sys_mount (regs=<optimized out>) at fs/namespace.c:4215
> #15 0xffffffff81328ced in x64_sys_call (regs=0xffff88804b6e3cc0, nr=0) at arch/x86/entry/syscall_64.c:37
> #16 0xffffffff8ac0015a in do_syscall_x64 (regs=0xffffc9000f37ff48, nr=165) at arch/x86/entry/syscall_64.c:63
> #17 do_syscall_64 (regs=0xffffc9000f37ff48, nr=165) at arch/x86/entry/syscall_64.c:94
> #18 0xffffffff81000130 in entry_SYSCALL_64 () at arch/x86/entry/entry_64.S:121
> #19 0x0000000000000000 in ?? ()
>
> Hence, I think the function to fix would be ocfs2_map_slot_buffers() i.e.
> the call to ocfs2_read_blocks() in it. I came up with following changes:
Why does the syzbot test code never trigger ocfs2_refresh_slot_info()?
According to the comment for LOCK_TYPE_REQUIRES_REFRESH, we know it requires DLM
AST callback, but syzbot only mounts & operates the ocfs2 volume in local mode.
For this specific bug, adding validation function into ocfs2_map_slot_buffers()
is correct. However, we should also add a validation action in
ocfs2_refresh_slot_info(). This is another way to read slot map data.
>
> diff --git a/fs/ocfs2/slot_map.c b/fs/ocfs2/slot_map.c
> index e544c704b583..299751617919 100644
> --- a/fs/ocfs2/slot_map.c
> +++ b/fs/ocfs2/slot_map.c
> @@ -332,6 +332,26 @@ int ocfs2_clear_slot(struct ocfs2_super *osb, int slot_num)
> return ocfs2_update_disk_slot(osb, osb->slot_info, slot_num);
> }
>
> +static int ocfs2_validate_slot_map_block(struct super_block *sb,
> + struct buffer_head *bh)
> +{
> + int rc;
> +
> + BUG_ON(!buffer_uptodate(bh));
> +
> + if (bh->b_blocknr < OCFS2_SUPER_BLOCK_BLKNO) {
> + rc = ocfs2_error(osb->sb,
> + "Invalid Slot Map Buffer Head "
> + "Block Number : %llu, Should be >= %d",
> + (unsigned long long)bh->b_blocknr,
> + OCFS2_SUPER_BLOCK_BLKNO);
> + if (!rc)
> + return -EIO;
> + return status;
> + }
> + return 0;
> +}
> +
> static int ocfs2_map_slot_buffers(struct ocfs2_super *osb,
> struct ocfs2_slot_info *si)
> {
> @@ -383,7 +403,8 @@ static int ocfs2_map_slot_buffers(struct ocfs2_super *osb,
>
> bh = NULL; /* Acquire a fresh bh */
> status = ocfs2_read_blocks(INODE_CACHE(si->si_inode), blkno,
> - 1, &bh, OCFS2_BH_IGNORE_CACHE, NULL);
> + 1, &bh, OCFS2_BH_IGNORE_CACHE,
> + ocfs2_validate_slot_map_block);
> if (status < 0) {
> mlog_errno(status);
> goto bail;
>
> What do you think? Is this the correct approach?
>
> Thank You,
> Prithvi
Based on my above comments, we should also add ocfs2_validate_slot_map_block()
in ocfs2_refresh_slot_info().
Thanks,
Heming
Powered by blists - more mailing lists