[PATCH] usercopy: Check valid lifetime via stack depth

[PATCH] usercopy: Check valid lifetime via stack depth

[Kees Cook]

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).

Additionally report usercopy bounds checking failures with an offset
from current_stack_pointer, which may assist with diagnosing failures.

The LKDTM USERCOPY_STACK_FRAME_TO and USERCOPY_STACK_FRAME_FROM tests
(once slightly adjusted in a separate patch) will pass again with
this fixed.

Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 mm/usercopy.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/mm/usercopy.c b/mm/usercopy.c
index d0d268135d96..3846c1634dca 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -29,13 +29,20 @@
  * Returns:
  *	NOT_STACK: not at all on the stack
  *	GOOD_FRAME: fully within a valid stack frame
- *	GOOD_STACK: fully on the stack (when can't do frame-checking)
+ *	GOOD_STACK: within the current stack (when can't frame-check exactly)
  *	BAD_STACK: error condition (invalid stack position or bad stack frame)
  */
 static noinline int check_stack_object(const void *obj, unsigned long len)
 {
 	const void * const stack = task_stack_page(current);
 	const void * const stackend = stack + THREAD_SIZE;
+#ifndef CONFIG_STACK_GROWSUP
+	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
 	int ret;
 
 	/* Object is not on the stack at all. */
@@ -55,6 +62,12 @@ static noinline int check_stack_object(const void *obj, unsigned long len)
 	if (ret)
 		return ret;
 
+	/*
+	 * Reject: object not within current stack depth.
+	 */
+	if (obj < low || high < obj + len)
+		return BAD_STACK;
+
 	return GOOD_STACK;
 }
 
@@ -280,7 +293,13 @@ void __check_object_size(const void *ptr, unsigned long n, bool to_user)
 		 */
 		return;
 	default:
-		usercopy_abort("process stack", NULL, to_user, 0, n);
+		usercopy_abort("process stack", NULL, to_user,
+#ifndef CONFIG_STACK_GROWSUP
+				(void *)current_stack_pointer - ptr,
+#else
+				ptr - (void *)current_stack_pointer,
+#endif
+				n);
 	}
 
 	/* Check for bad heap object. */
-- 
2.30.2

Re: [PATCH] usercopy: Check valid lifetime via stack depth

[kernel test robot]

Hi Kees,

I love your patch! Yet something to improve:

[auto build test ERROR on hnaz-mm/master]
[also build test ERROR on kees/for-next/pstore v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kees-Cook/usercopy-Check-valid-lifetime-via-stack-depth/20220217-041611
base:   https://github.com/hnaz/linux-mm master
config: openrisc-randconfig-r002-20220216 (https://download.01.org/0day-ci/archive/20220217/202202170844.jnpFFEmh-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/77944e5fa0cf5a29903b72466a22152c6a5d41ac
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kees-Cook/usercopy-Check-valid-lifetime-via-stack-depth/20220217-041611
        git checkout 77944e5fa0cf5a29903b72466a22152c6a5d41ac
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=openrisc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   mm/usercopy.c: In function 'check_stack_object':
>> mm/usercopy.c:41:42: error: 'current_stack_pointer' undeclared (first use in this function); did you mean 'user_stack_pointer'?
      41 |         const void * const low = (void *)current_stack_pointer;
         |                                          ^~~~~~~~~~~~~~~~~~~~~
         |                                          user_stack_pointer
   mm/usercopy.c:41:42: note: each undeclared identifier is reported only once for each function it appears in
   mm/usercopy.c: In function '__check_object_size':
   mm/usercopy.c:287:41: error: 'current_stack_pointer' undeclared (first use in this function); did you mean 'user_stack_pointer'?
     287 |                                 (void *)current_stack_pointer - ptr,
         |                                         ^~~~~~~~~~~~~~~~~~~~~
         |                                         user_stack_pointer


vim +41 mm/usercopy.c

    24	
    25	/*
    26	 * Checks if a given pointer and length is contained by the current
    27	 * stack frame (if possible).
    28	 *
    29	 * Returns:
    30	 *	NOT_STACK: not at all on the stack
    31	 *	GOOD_FRAME: fully within a valid stack frame
    32	 *	GOOD_STACK: within the current stack (when can't frame-check exactly)
    33	 *	BAD_STACK: error condition (invalid stack position or bad stack frame)
    34	 */
    35	static noinline int check_stack_object(const void *obj, unsigned long len)
    36	{
    37		const void * const stack = task_stack_page(current);
    38		const void * const stackend = stack + THREAD_SIZE;
    39	#ifndef CONFIG_STACK_GROWSUP
    40		const void * const high = stackend;
  > 41		const void * const low = (void *)current_stack_pointer;
    42	#else
    43		const void * const high = (void *)current_stack_pointer;
    44		const void * const low = stack;
    45	#endif
    46		int ret;
    47	
    48		/* Object is not on the stack at all. */
    49		if (obj + len <= stack || stackend <= obj)
    50			return NOT_STACK;
    51	
    52		/*
    53		 * Reject: object partially overlaps the stack (passing the
    54		 * check above means at least one end is within the stack,
    55		 * so if this check fails, the other end is outside the stack).
    56		 */
    57		if (obj < stack || stackend < obj + len)
    58			return BAD_STACK;
    59	
    60		/* Check if object is safely within a valid frame. */
    61		ret = arch_within_stack_frames(stack, stackend, obj, len);
    62		if (ret)
    63			return ret;
    64	
    65		/*
    66		 * Reject: object not within current stack depth.
    67		 */
    68		if (obj < low || high < obj + len)
    69			return BAD_STACK;
    70	
    71		return GOOD_STACK;
    72	}
    73	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH] usercopy: Check valid lifetime via stack depth

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4474 bytes --]

Hi Kees,

I love your patch! Yet something to improve:

[auto build test ERROR on hnaz-mm/master]
[also build test ERROR on kees/for-next/pstore v5.17-rc4 next-20220216]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Kees-Cook/usercopy-Check-valid-lifetime-via-stack-depth/20220217-041611
base:   https://github.com/hnaz/linux-mm master
config: openrisc-randconfig-r002-20220216 (https://download.01.org/0day-ci/archive/20220217/202202170844.jnpFFEmh-lkp(a)intel.com/config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/77944e5fa0cf5a29903b72466a22152c6a5d41ac
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Kees-Cook/usercopy-Check-valid-lifetime-via-stack-depth/20220217-041611
        git checkout 77944e5fa0cf5a29903b72466a22152c6a5d41ac
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=openrisc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   mm/usercopy.c: In function 'check_stack_object':
>> mm/usercopy.c:41:42: error: 'current_stack_pointer' undeclared (first use in this function); did you mean 'user_stack_pointer'?
      41 |         const void * const low = (void *)current_stack_pointer;
         |                                          ^~~~~~~~~~~~~~~~~~~~~
         |                                          user_stack_pointer
   mm/usercopy.c:41:42: note: each undeclared identifier is reported only once for each function it appears in
   mm/usercopy.c: In function '__check_object_size':
   mm/usercopy.c:287:41: error: 'current_stack_pointer' undeclared (first use in this function); did you mean 'user_stack_pointer'?
     287 |                                 (void *)current_stack_pointer - ptr,
         |                                         ^~~~~~~~~~~~~~~~~~~~~
         |                                         user_stack_pointer


vim +41 mm/usercopy.c

    24	
    25	/*
    26	 * Checks if a given pointer and length is contained by the current
    27	 * stack frame (if possible).
    28	 *
    29	 * Returns:
    30	 *	NOT_STACK: not at all on the stack
    31	 *	GOOD_FRAME: fully within a valid stack frame
    32	 *	GOOD_STACK: within the current stack (when can't frame-check exactly)
    33	 *	BAD_STACK: error condition (invalid stack position or bad stack frame)
    34	 */
    35	static noinline int check_stack_object(const void *obj, unsigned long len)
    36	{
    37		const void * const stack = task_stack_page(current);
    38		const void * const stackend = stack + THREAD_SIZE;
    39	#ifndef CONFIG_STACK_GROWSUP
    40		const void * const high = stackend;
  > 41		const void * const low = (void *)current_stack_pointer;
    42	#else
    43		const void * const high = (void *)current_stack_pointer;
    44		const void * const low = stack;
    45	#endif
    46		int ret;
    47	
    48		/* Object is not on the stack at all. */
    49		if (obj + len <= stack || stackend <= obj)
    50			return NOT_STACK;
    51	
    52		/*
    53		 * Reject: object partially overlaps the stack (passing the
    54		 * check above means@least one end is within the stack,
    55		 * so if this check fails, the other end is outside the stack).
    56		 */
    57		if (obj < stack || stackend < obj + len)
    58			return BAD_STACK;
    59	
    60		/* Check if object is safely within a valid frame. */
    61		ret = arch_within_stack_frames(stack, stackend, obj, len);
    62		if (ret)
    63			return ret;
    64	
    65		/*
    66		 * Reject: object not within current stack depth.
    67		 */
    68		if (obj < low || high < obj + len)
    69			return BAD_STACK;
    70	
    71		return GOOD_STACK;
    72	}
    73	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org