On Wed, Aug 17, 2022 at 3:17 PM Andrew Price <anpr...@redhat.com> wrote:
> Fuzzers like to scribble over sb_bsize_shift but in reality it's very
> unlikely that this field would be corrupted on its own. Nevertheless it
> should be checked to avoid the possibility of messy mount errors due to
> bad calculations. It's always a fixed value based on the block size so
> we can just check that it's the expected value.
>
> Tested with:
>
> mkfs.gfs2 -O -p lock_nolock /dev/vdb
> for i in 0 -1 64 65 32 33; do
> gfs2_edit -p sb field sb_bsize_shift $i /dev/vdb
> mount /dev/vdb /mnt/test && umount /mnt/test
> done
>
> Before this patch we get a withdraw after
>
> [ 76.413681] gfs2: fsid=loop0.0: fatal: invalid metadata block
> [ 76.413681] bh = 19 (type: exp=5, found=4)
> [ 76.413681] function = gfs2_meta_buffer, file = fs/gfs2/meta_io.c, line
> = 492
>
> and with UBSAN configured we also get complaints like
>
> [ 76.373395] UBSAN: shift-out-of-bounds in fs/gfs2/ops_fstype.c:295:19
> [ 76.373815] shift exponent 4294967287 is too large for 64-bit type 'long
> unsigned int'
>
> After the patch, these complaints don't appear, mount fails immediately
> and we get an explanation in dmesg.
>
> Reported-by: syzbot+dcf33a7aae997956f...@syzkaller.appspotmail.com
> Signed-off-by: Andrew Price <anpr...@redhat.com>
> ---
> fs/gfs2/ops_fstype.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index 549879929c84..692e27f8f563 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -178,7 +178,10 @@ static int gfs2_check_sb(struct gfs2_sbd *sdp, int
> silent)
> pr_warn("Invalid block size\n");
> return -EINVAL;
> }
> -
> + if (sb->sb_bsize_shift != ffs(sb->sb_bsize) - 1) {
> + pr_warn("Invalid block size shift\n");
> + return -EINVAL;
> + }
> return 0;
> }
>
> --
> 2.37.1
Added now; sorry for overlooking this.
Thanks,
Andreas
