lkml.org 
[lkml]   [2019]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: WARNING: bad usercopy in fanotify_read
On Mon, Mar 11, 2019 at 1:42 PM syzbot
<syzbot+2c49971e251e36216d1f@syzkaller.appspotmail.com> wrote:
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17ee410b200000
> [...]
> ------------[ cut here ]------------
> Bad or missing usercopy whitelist? Kernel memory exposure attempt detected
> from SLAB object 'fanotify_event' (offset 40, size 8)!
> [...]
> copy_to_user include/linux/uaccess.h:151 [inline]
> copy_fid_to_user fs/notify/fanotify/fanotify_user.c:236 [inline]
> copy_event_to_user fs/notify/fanotify/fanotify_user.c:294 [inline]

Looks like this is the fh/ext_fh union in struct fanotify_fid, field
"fid" in struct fanotify_event. Given that "fid" is itself in a union
against a struct path, I think instead of a whitelist using
KMEM_CACHE_USERCOPY(), this should just use a bounce buffer to avoid
leaving a whitelist open for path or ext_fh exposure.

Maybe something like this (untested):

diff --git a/fs/notify/fanotify/fanotify_user.c
b/fs/notify/fanotify/fanotify_user.c
index 56992b32c6bb..b87da9580b3c 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -207,6 +207,7 @@ static int process_access_response(struct
fsnotify_group *group,
static int copy_fid_to_user(struct fanotify_event *event, char __user *buf)
{
struct fanotify_event_info_fid info = { };
+ unsigned char bounce[FANOTIFY_INLINE_FH_LEN], *fh;
struct file_handle handle = { };
size_t fh_len = event->fh_len;
size_t len = fanotify_event_info_len(event);
@@ -233,7 +234,18 @@ static int copy_fid_to_user(struct fanotify_event
*event, char __user *buf)

buf += sizeof(handle);
len -= sizeof(handle);
- if (copy_to_user(buf, fanotify_event_fh(event), fh_len))
+
+ /*
+ * For an inline fh, copy through stack to exclude the copy from
+ * usercopy hardening protections.
+ */
+ fh = fanotify_event_fh(event);
+ if (fh_len <= sizeof(bounce)) {
+ memcpy(bounce, fh, fh_len);
+ fh = bounce;
+ }
+
+ if (copy_to_user(buf, fh, fh_len))
return -EFAULT;

/* Pad with 0's */

--
Kees Cook

\
 
 \ /
  Last update: 2019-03-13 07:27    [W:0.070 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site