syz report a shift-out-of-bounds in v9fs_get_tree.

This is because the maxdata value is 0, causing fls to return 32, meaning
the s_blocksize_bits value is 32, which causes an out of bounds error.
The root cause of this is incorrect session information obtained during
fill super. Since v9ses is stored in sb, it is used directly.

Fixes: 4d18c32a395d ("9p: convert to the new mount API")
Reported-by: syzbot+30c83da54e948f6e9436@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=30c83da54e948f6e9436
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: remove the unused ctx

 fs/9p/vfs_super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index f6065b5e0e5d..bcb6ebdb8037 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -49,8 +49,7 @@ static int v9fs_set_super(struct super_block *s, struct fs_context *fc)
 static int v9fs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	int ret;
-	struct v9fs_context	*ctx = fc->fs_private;
-	struct v9fs_session_info *v9ses = &ctx->v9ses;
+	struct v9fs_session_info *v9ses = sb->s_fs_info;
 
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_blocksize_bits = fls(v9ses->maxdata - 1);
-- 
2.43.0

Edward Adam Davis wrote on Sat, Aug 23, 2025 at 07:22:13AM +0800:
> syz report a shift-out-of-bounds in v9fs_get_tree.
> 
> This is because the maxdata value is 0, causing fls to return 32, meaning
> the s_blocksize_bits value is 32, which causes an out of bounds error.
> The root cause of this is incorrect session information obtained during
> fill super. Since v9ses is stored in sb, it is used directly.

Thanks for the patch.

Eric, ignore the other part of the thread -- I guess the int max limit
wasn't related...

What I'm not following now is how the v9ses is created/handled around
the new mount API:
- in v9fs_get_tree a v9ses is allocated and passed along in
fc->s_fs_info (that this patches now uses)
- but in v9fs_init_fs_context then a `v9fs_context` is created that
also embeds (not a pointer) a v9ses struct, which is accessed through
fc->fs_private as the code before this patch.

So at least for some time we have two v9ses which obviously don't hold
the same fields, and I'm not confident I can review which is used where
and when.

Now I probably should read up about the "new" mount API, but I don't
like that there are two v9ses around.
I don't have a clue about the fs_context lifetime: is it kept around all
the time the fs is mounted and can we rely on it to be present (and get
rid of the v9ses allocated in v9fs_get_tree), or is the context only a
temporary thing and we should avoid having a v9ses in there instead?
(I'd be tempted to think the later?)


Edward, thanks for investingating this; at this point I'm worried there
are other inconsistencies so I'll just remove the new mount API patches
from my -next branch instead of applying the patch, but this is really
appreciated.

> Fixes: 4d18c32a395d ("9p: convert to the new mount API")
> Reported-by: syzbot+30c83da54e948f6e9436@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=30c83da54e948f6e9436
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: remove the unused ctx
> 
>  fs/9p/vfs_super.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> index f6065b5e0e5d..bcb6ebdb8037 100644
> --- a/fs/9p/vfs_super.c
> +++ b/fs/9p/vfs_super.c
> @@ -49,8 +49,7 @@ static int v9fs_set_super(struct super_block *s, struct fs_context *fc)
>  static int v9fs_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
>  	int ret;
> -	struct v9fs_context	*ctx = fc->fs_private;
> -	struct v9fs_session_info *v9ses = &ctx->v9ses;
> +	struct v9fs_session_info *v9ses = sb->s_fs_info;
>  
>  	sb->s_maxbytes = MAX_LFS_FILESIZE;
>  	sb->s_blocksize_bits = fls(v9ses->maxdata - 1);

-- 
Dominique Martinet | Asmadeus