On Sat, May 23, 2020 at 4:22 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> syzbot reported out of bounds memory access from open_by_handle_at()
> with a crafted file handle that looks like this:
>
> { .handle_bytes = 2, .handle_type = OVL_FILEID_V1 }
>
> handle_bytes gets rounded down to 0 and we end up calling:
> ovl_check_fh_len(fh, 0) => ovl_check_fb_len(fh + 3, -3)
>
> But fh buffer is only 2 bytes long, so accessing struct ovl_fb at
> fh + 3 is illegal.
>
> Fixes: cbe7fba8edfc ("ovl: make sure that real fid is 32bit aligned in memory")
> Reported-and-tested-by: syzbot+61958888b1c60361a791@xxxxxxxxxxxxxxxxxxxxxxxxx
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.5
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>
> Miklos,
>
Ping.
> Another fallout from aligned file handle.
> This one seems like a warning that cannot lead to actual harm.
> As far as I can tell, with:
>
> { .handle_bytes = 2, .handle_type = OVL_FILEID_V1 }
>
> kmalloc in handle_to_path() allocates 10 bytes, which means 16 bytes
> slab object, so all fields accessed by ovl_check_fh_len() should be
> within the slab object boundaries. And in any case, their value
> won't change the outcome of EINVAL.
>
> I have added this use case to the xfstest for checking the first bug,
> but it doesn't trigger any warning on my kernel (without KASAN) and
> returns EINVAL as expected.
>
> Thanks,
> Amir.
>
> fs/overlayfs/overlayfs.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 76747f5b0517..ffbb57b2d7f6 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -355,6 +355,9 @@ int ovl_check_fb_len(struct ovl_fb *fb, int fb_len);
>
> static inline int ovl_check_fh_len(struct ovl_fh *fh, int fh_len)
> {
> + if (fh_len < sizeof(struct ovl_fh))
> + return -EINVAL;
> +
> return ovl_check_fb_len(&fh->fb, fh_len - OVL_FH_WIRE_OFFSET);
> }
>
> --
> 2.17.1
>
