Re: [PATCH] fs/proc/kcore.c: Omit kernel text area for hardened usercopy feature

[Jiri Olsa]

On Fri, Sep 02, 2016 at 08:17:13AM -0700, Andi Kleen wrote:
> On Fri, Sep 02, 2016 at 02:25:45PM +0200, Jiri Olsa wrote:
> > One of the bullets for hardened usercopy feature is:
> >   - object must not overlap with kernel text
> > 
> > which is what we expose via /proc/kcore. We can hit
> > this check and crash the system very easily just by
> > reading the text area in kcore file:
> > 
> >   usercopy: kernel memory exposure attempt detected from ffffffff8179a01f (<kernel text>) (4065 bytes)
> >   kernel BUG at mm/usercopy.c:75!
> > 
> > Omitting kernel text area from kcore when there's
> > hardened usercopy feature is enabled.
> 
> That will completely break PT decoding, which relies on looking
> at the kernel text in /proc/kcore.
> 
> Need a different fix here, perhaps some special copy function
> that is not hardened.

how about something like this

jirka


---
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index c3f291195294..43f5404f0e61 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -726,7 +726,8 @@ copy_from_user(void *to, const void __user *from, unsigned long n)
 }
 
 static inline unsigned long __must_check
-copy_to_user(void __user *to, const void *from, unsigned long n)
+copy_to_user_check(void __user *to, const void *from,
+		   unsigned long n, bool check)
 {
 	int sz = __compiletime_object_size(from);
 
@@ -735,7 +736,8 @@ copy_to_user(void __user *to, const void *from, unsigned long n)
 	might_fault();
 
 	if (likely(sz < 0 || sz >= n)) {
-		check_object_size(from, n, true);
+		if (check)
+			check_object_size(from, n, true);
 		n = _copy_to_user(to, from, n);
 	} else if (!__builtin_constant_p(n))
 		copy_user_overflow(sz, n);
@@ -745,6 +747,19 @@ copy_to_user(void __user *to, const void *from, unsigned long n)
 	return n;
 }
 
+static inline unsigned long __must_check
+copy_to_user(void __user *to, const void *from, unsigned long n)
+{
+	return copy_to_user_check(to, from, n, true);
+}
+
+static inline unsigned long __must_check
+copy_to_user_nocheck(void __user *to, const void *from, unsigned long n)
+{
+	return copy_to_user_check(to, from, n, false);
+}
+
+
 /*
  * We rely on the nested NMI work to allow atomic faults from the NMI path; the
  * nested NMI paths are careful to preserve CR2.
diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
index 673059a109fe..e80e4a146b7d 100644
--- a/arch/x86/include/asm/uaccess_64.h
+++ b/arch/x86/include/asm/uaccess_64.h
@@ -50,7 +50,7 @@ __must_check unsigned long
 copy_in_user(void __user *to, const void __user *from, unsigned len);
 
 static __always_inline __must_check
-int __copy_from_user_nocheck(void *dst, const void __user *src, unsigned size)
+int __copy_from_user_nofaultcheck(void *dst, const void __user *src, unsigned size)
 {
 	int ret = 0;
 
@@ -112,7 +112,7 @@ int __copy_from_user(void *dst, const void __user *src, unsigned size)
 {
 	might_fault();
 	kasan_check_write(dst, size);
-	return __copy_from_user_nocheck(dst, src, size);
+	return __copy_from_user_nofaultcheck(dst, src, size);
 }
 
 static __always_inline __must_check
@@ -248,7 +248,7 @@ static __must_check __always_inline int
 __copy_from_user_inatomic(void *dst, const void __user *src, unsigned size)
 {
 	kasan_check_write(dst, size);
-	return __copy_from_user_nocheck(dst, src, size);
+	return __copy_from_user_nofaultcheck(dst, src, size);
 }
 
 static __must_check __always_inline int
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index a939f5ed7f89..c7a22a8a157e 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -516,7 +516,7 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
 			if (kern_addr_valid(start)) {
 				unsigned long n;
 
-				n = copy_to_user(buffer, (char *)start, tsz);
+				n = copy_to_user_nocheck(buffer, (char *)start, tsz);
 				/*
 				 * We cannot distinguish between fault on source
 				 * and fault on destination. When this happens