Re: [syzbot] [v9fs?] UBSAN: shift-out-of-bounds in v9fs_get_tree

Eric Sandeen
2025-09-24T23:14:54Z

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