linux-kernel - Re: [syzbot] [v9fs?] UBSAN: shift-out-of-bounds in v9fs_get

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <24cb212f-fac1-48c2-8418-0160f92d9d84@redhat.com>
Date: Wed, 24 Sep 2025 18:14:47 -0500
From: Eric Sandeen <sandeen@...hat.com>
To: Dominique Martinet <asmadeus@...ewreck.org>
Cc: ericvh@...nel.org, linux-kernel@...r.kernel.org, linux_oss@...debyte.com,
 lucho@...kov.net, syzkaller-bugs@...glegroups.com, v9fs@...ts.linux.dev,
 syzbot <syzbot+30c83da54e948f6e9436@...kaller.appspotmail.com>
Subject: Re: [syzbot] [v9fs?] UBSAN: shift-out-of-bounds in v9fs_get_tree

On 8/20/25 10:48 PM, Dominique Martinet wrote:
> Hi Eric,

Again, apologies, not sure how I missed this as well.

But circling back:

> syzbot wrote on Wed, Aug 20, 2025 at 07:58:31PM -0700:
>> UBSAN: shift-out-of-bounds in fs/9p/vfs_super.c:57:22
>> shift exponent 32 is too large for 32-bit type 'int'
>> Call Trace:
>>  <TASK>
>>  dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
>>  ubsan_epilogue+0xa/0x40 lib/ubsan.c:233
>>  __ubsan_handle_shift_out_of_bounds+0x386/0x410 lib/ubsan.c:494
>>  v9fs_fill_super fs/9p/vfs_super.c:57 [inline]
>>  v9fs_get_tree+0x957/0xa90 fs/9p/vfs_super.c:125
>>  vfs_get_tree+0x8f/0x2b0 fs/super.c:1752
>>  do_new_mount+0x2a2/0xa30 fs/namespace.c:3810
>>  do_mount fs/namespace.c:4138 [inline]
> 
> I thinks the mount rework triggered this one (full copy below or at [1])
> [1] https://lore.kernel.org/all/68a68b57.050a0220.3d78fd.0012.GAE@google.com/T/#u
> 
> From a quick look the old code bound msize to 4k-INT_MAX, but the new
> code accepts higher uint32 values.
> To be honest I'm not sure INT_MAX even makes sense as later allocations
> are likely to work :) but for now something as simple as this is likely
> to work (I'm not sure I got the test thing right, let's see...)
> 
> Shall I just roll that into your patch, unless you know of a more
> appropriate limit?
> There doesn't seem to be any easy to use variable about max allocation
> size, a limit of a few MB is probably sensible but I don't like
> artificial restrictions just to please syzbot so happy to defer to
> someone else here.
> 
> 
> #syz test
> 
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index 55ba26186351..cc65330ee684 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -302,6 +302,10 @@ int v9fs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>                         p9_debug(P9_DEBUG_ERROR, "msize should be at least 4k\n");
>                         return -EINVAL;
>                 }
> +               if (result.uint_32 > INT_MAX) {
> +                       p9_debug(P9_DEBUG_ERROR, "msize too big\n");
> +                       return -EINVAL;
> +               }

FWIW if we need to limit msize to a signed int, we can just change the
Opt_msize entry in v9fs_param_spec[] to an fsparam_s32 and anything
bigger should be rejected by the core parsers. The parsed value would be
retrieved via result.int_32 (vs. result.uint_32 here).

(I had seen {Opt_msize, "msize=%u"} and thought "unsigned" but missed
that it actually used match_int(). So probably a couple other spots
diverged with my patch as well, though maybe they are of less
consequence.)

-Eric