[02/38] usercopy: Enhance and rename report_usercopy()
diff mbox

Message ID 1515636190-24061-3-git-send-email-keescook@chromium.org
State New
Headers show

Commit Message

Kees Cook Jan. 11, 2018, 2:02 a.m. UTC
In preparation for refactoring the usercopy checks to pass offset to
the hardened usercopy report, this renames report_usercopy() to the
more accurate usercopy_abort(), marks it as noreturn because it is,
adds a hopefully helpful comment for anyone investigating such reports,
makes the function available to the slab allocators, and adds new "detail"
and "offset" arguments.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 mm/slab.h             |  6 ++++++
 mm/usercopy.c         | 24 +++++++++++++++++++-----
 tools/objtool/check.c |  1 +
 3 files changed, 26 insertions(+), 5 deletions(-)

Comments

Christopher Lameter Jan. 11, 2018, 5:06 p.m. UTC | #1
On Wed, 10 Jan 2018, Kees Cook wrote:

> diff --git a/mm/slab.h b/mm/slab.h
> index ad657ffa44e5..7d29e69ac310 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -526,4 +526,10 @@ static inline int cache_random_seq_create(struct kmem_cache *cachep,
>  static inline void cache_random_seq_destroy(struct kmem_cache *cachep) { }
>  #endif /* CONFIG_SLAB_FREELIST_RANDOM */
>
> +#ifdef CONFIG_HARDENED_USERCOPY
> +void __noreturn usercopy_abort(const char *name, const char *detail,
> +			       bool to_user, unsigned long offset,
> +			       unsigned long len);
> +#endif
> +
>  #endif /* MM_SLAB_H */

This code has nothing to do with slab allocation. Move it into
include/linux/uaccess.h where the other user space access definitions are?
Kees Cook Jan. 14, 2018, 8:57 p.m. UTC | #2
On Thu, Jan 11, 2018 at 9:06 AM, Christopher Lameter <cl@linux.com> wrote:
> On Wed, 10 Jan 2018, Kees Cook wrote:
>
>> diff --git a/mm/slab.h b/mm/slab.h
>> index ad657ffa44e5..7d29e69ac310 100644
>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -526,4 +526,10 @@ static inline int cache_random_seq_create(struct kmem_cache *cachep,
>>  static inline void cache_random_seq_destroy(struct kmem_cache *cachep) { }
>>  #endif /* CONFIG_SLAB_FREELIST_RANDOM */
>>
>> +#ifdef CONFIG_HARDENED_USERCOPY
>> +void __noreturn usercopy_abort(const char *name, const char *detail,
>> +                            bool to_user, unsigned long offset,
>> +                            unsigned long len);
>> +#endif
>> +
>>  #endif /* MM_SLAB_H */
>
> This code has nothing to do with slab allocation. Move it into
> include/linux/uaccess.h where the other user space access definitions are?

Since it was only the mm/sl*b.c files using it, it seemed like the
right place, but it's a reasonable point. I've moved it now.

-Kees

Patch
diff mbox

diff --git a/mm/slab.h b/mm/slab.h
index ad657ffa44e5..7d29e69ac310 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -526,4 +526,10 @@  static inline int cache_random_seq_create(struct kmem_cache *cachep,
 static inline void cache_random_seq_destroy(struct kmem_cache *cachep) { }
 #endif /* CONFIG_SLAB_FREELIST_RANDOM */
 
+#ifdef CONFIG_HARDENED_USERCOPY
+void __noreturn usercopy_abort(const char *name, const char *detail,
+			       bool to_user, unsigned long offset,
+			       unsigned long len);
+#endif
+
 #endif /* MM_SLAB_H */
diff --git a/mm/usercopy.c b/mm/usercopy.c
index 5df1e68d4585..8006baa4caac 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -58,11 +58,25 @@  static noinline int check_stack_object(const void *obj, unsigned long len)
 	return GOOD_STACK;
 }
 
-static void report_usercopy(unsigned long len, bool to_user, const char *type)
+/*
+ * If this function is reached, then CONFIG_HARDENED_USERCOPY has found an
+ * unexpected state during a copy_from_user() or copy_to_user() call.
+ * There are several checks being performed on the buffer by the
+ * __check_object_size() function. Normal stack buffer usage should never
+ * trip the checks, and kernel text addressing will always trip the check.
+ * For cache objects, copies must be within the object size.
+ */
+void __noreturn usercopy_abort(const char *name, const char *detail,
+			       bool to_user, unsigned long offset,
+			       unsigned long len)
 {
-	pr_emerg("kernel memory %s attempt detected %s '%s' (%lu bytes)\n",
-		to_user ? "exposure" : "overwrite",
-		to_user ? "from" : "to", type ? : "unknown", len);
+	pr_emerg("Kernel memory %s attempt detected %s %s%s%s%s (offset %lu, size %lu)!\n",
+		 to_user ? "exposure" : "overwrite",
+		 to_user ? "from" : "to",
+		 name ? : "unknown?!",
+		 detail ? " '" : "", detail ? : "", detail ? "'" : "",
+		 offset, len);
+
 	/*
 	 * For greater effect, it would be nice to do do_group_exit(),
 	 * but BUG() actually hooks all the lock-breaking and per-arch
@@ -260,6 +274,6 @@  void __check_object_size(const void *ptr, unsigned long n, bool to_user)
 		return;
 
 report:
-	report_usercopy(n, to_user, err);
+	usercopy_abort(err, NULL, to_user, 0, n);
 }
 EXPORT_SYMBOL(__check_object_size);
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 9b341584eb1b..ae39444896d4 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -138,6 +138,7 @@  static int __dead_end_function(struct objtool_file *file, struct symbol *func,
 		"__reiserfs_panic",
 		"lbug_with_loc",
 		"fortify_panic",
+		"usercopy_abort",
 	};
 
 	if (func->bind == STB_WEAK)