[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-unionfs
Subject:    [PATCH] ovl: fix out of bounds access warning in ovl_check_fb_len()
From:       Amir Goldstein <amir73il () gmail ! com>
Date:       2020-05-23 13:21:55
Message-ID: 20200523132155.14698-1-amir73il () gmail ! com
[Download RAW message or body]

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@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org> # v5.5
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Miklos,

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

[prev in list] [next in list] [prev in thread] [next in thread]