On Thu, Feb 24, 2022 at 08:58:20AM +0000, David Laight wrote:

From: Kees Cook

quoted

Sent: 24 February 2022 06:04

Under CONFIG_HARDENED_USERCOPY=y, when exact stack frame boundary checking
is not available (i.e. everything except x86 with FRAME_POINTER), check
a stack object as being at least "current depth valid", in the sense
that any object within the stack region but not between start-of-stack
and current_stack_pointer should be considered unavailable (i.e. its
lifetime is from a call no longer present on the stack).

...

quoted

diff --git a/mm/usercopy.c b/mm/usercopy.c
index d0d268135d96..5d28725af95f 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c

@@ -22,6 +22,30 @@
 #include <asm/sections.h>
 #include "slab.h"

+/*
+ * Only called if obj is within stack/stackend bounds. Determine if within
+ * current stack depth.
+ */
+static inline int check_stack_object_depth(const void *obj,
+					   unsigned long len)
+{
+#ifdef CONFIG_ARCH_HAS_CURRENT_STACK_POINTER
+#ifndef CONFIG_STACK_GROWSUP

Pointless negation

quoted

+	const void * const high = stackend;
+	const void * const low = (void *)current_stack_pointer;
+#else
+	const void * const high = (void *)current_stack_pointer;
+	const void * const low = stack;
+#endif
+
+	/* Reject: object not within current stack depth. */
+	if (obj < low || high < obj + len)
+		return BAD_STACK;
+
+#endif
+	return GOOD_STACK;
+}

If the comment at the top of the function is correct then
only a single test for the correct end of the buffer against
the current stack pointer is needed.
Something like:
#ifdef CONFIG_STACK_GROWSUP
	if ((void *)current_stack_pointer < obj + len)
		return BAD_STACK;
#else
	if (obj < (void *)current_stack_pointer)
		return BAD_STACK;
#endif
	return GOOD_STACK;

Oh, yeah, excellent point. I suspect the compiler would probably
optimize it all away, but yes, this is, in fact, easier to read, and
short enough I should probably just not bother with a separate function.

Thanks!

-Kees

Although it may depend on exactly where the stack pointer
points to - especially for GROWSUP.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

-- 
Kees Cook