]> git.ipfire.org Git - people/ms/linux.git/commitdiff
usercopy: Check valid lifetime via stack depth
authorKees Cook <keescook@chromium.org>
Wed, 16 Feb 2022 20:05:28 +0000 (12:05 -0800)
committerKees Cook <keescook@chromium.org>
Sat, 26 Feb 2022 02:20:11 +0000 (18:20 -0800)
One of the things that CONFIG_HARDENED_USERCOPY sanity-checks is whether
an object that is about to be copied to/from userspace is overlapping
the stack at all. If it is, it performs a number of inexpensive
bounds checks. One of the finer-grained checks is whether an object
crosses stack frames within the stack region. Doing this on x86 with
CONFIG_FRAME_POINTER was cheap/easy. Doing it with ORC was deemed too
heavy, and was left out (a while ago), leaving the courser whole-stack
check.

The LKDTM tests USERCOPY_STACK_FRAME_TO and USERCOPY_STACK_FRAME_FROM
try to exercise these cross-frame cases to validate the defense is
working. They have been failing ever since ORC was added (which was
expected). While Muhammad was investigating various LKDTM failures[1],
he asked me for additional details on them, and I realized that when
exact stack frame boundary checking is not available (i.e. everything
except x86 with FRAME_POINTER), it could check if a stack object is 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).

Introduce ARCH_HAS_CURRENT_STACK_POINTER to track which architectures
have actually implemented the common global register alias.

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) pass again with this fixed.

[1] https://github.com/kernelci/kernelci-project/issues/84

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
Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v1: https://lore.kernel.org/lkml/20220216201449.2087956-1-keescook@chromium.org
v2: https://lore.kernel.org/lkml/20220224060342.1855457-1-keescook@chromium.org
v3: https://lore.kernel.org/lkml/20220225173345.3358109-1-keescook@chromium.org
v4: - improve commit log (akpm)

arch/arm/Kconfig
arch/arm64/Kconfig
arch/powerpc/Kconfig
arch/s390/Kconfig
arch/sh/Kconfig
arch/x86/Kconfig
mm/Kconfig
mm/usercopy.c

index 4c97cb40eebb632eafc036ee6d4f8c4dce570b35..a7a09eef1852157639ae40e86a1b1c1e213954e5 100644 (file)
@@ -5,6 +5,7 @@ config ARM
        select ARCH_32BIT_OFF_T
        select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE if HAVE_KRETPROBES && FRAME_POINTER && !ARM_UNWIND
        select ARCH_HAS_BINFMT_FLAT
+       select ARCH_HAS_CURRENT_STACK_POINTER
        select ARCH_HAS_DEBUG_VIRTUAL if MMU
        select ARCH_HAS_DMA_WRITE_COMBINE if !ARM_DMA_MEM_BUFFERABLE
        select ARCH_HAS_ELF_RANDOMIZE
index f2b5a4abef215275a73581e2a9bc7f1fe817eea1..b8ab790555c81ad9b00cdbcf5151085e8f7f1214 100644 (file)
@@ -18,6 +18,7 @@ config ARM64
        select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
        select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
        select ARCH_HAS_CACHE_LINE_SIZE
+       select ARCH_HAS_CURRENT_STACK_POINTER
        select ARCH_HAS_DEBUG_VIRTUAL
        select ARCH_HAS_DEBUG_VM_PGTABLE
        select ARCH_HAS_DMA_PREP_COHERENT
index b779603978e10017dcfed203654b0f3cf34ad2ba..7e7387bd7d538221ae1f6741a51b0f18a217ed92 100644 (file)
@@ -108,6 +108,7 @@ config PPC
        select ARCH_ENABLE_MEMORY_HOTPLUG
        select ARCH_ENABLE_MEMORY_HOTREMOVE
        select ARCH_HAS_COPY_MC                 if PPC64
+       select ARCH_HAS_CURRENT_STACK_POINTER
        select ARCH_HAS_DEBUG_VIRTUAL
        select ARCH_HAS_DEBUG_VM_PGTABLE
        select ARCH_HAS_DEBUG_WX                if STRICT_KERNEL_RWX
index be9f39fd06df6741e28ae08333b819150873ea46..4845ab549dd1d36f1fc8e3f21528856cf42ff0b5 100644 (file)
@@ -60,6 +60,7 @@ config S390
        select ARCH_ENABLE_MEMORY_HOTPLUG if SPARSEMEM
        select ARCH_ENABLE_MEMORY_HOTREMOVE
        select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2
+       select ARCH_HAS_CURRENT_STACK_POINTER
        select ARCH_HAS_DEBUG_VM_PGTABLE
        select ARCH_HAS_DEBUG_WX
        select ARCH_HAS_DEVMEM_IS_ALLOWED
index 2474a04ceac438975fcb0acb8daf730fbbb1edbe..1c2b53bf30930c9f911b03812b26c8958aa7b17f 100644 (file)
@@ -7,6 +7,7 @@ config SUPERH
        select ARCH_HAVE_CUSTOM_GPIO_H
        select ARCH_HAVE_NMI_SAFE_CMPXCHG if (GUSA_RB || CPU_SH4A)
        select ARCH_HAS_BINFMT_FLAT if !MMU
+       select ARCH_HAS_CURRENT_STACK_POINTER
        select ARCH_HAS_GIGANTIC_PAGE
        select ARCH_HAS_GCOV_PROFILE_ALL
        select ARCH_HAS_PTE_SPECIAL
index 9f5bd41bf660c7b024e6cf0838e6b7a3cf31b54e..90494fba3620fbbaff4aaabce0d2fb8a6d0120a4 100644 (file)
@@ -69,6 +69,7 @@ config X86
        select ARCH_ENABLE_THP_MIGRATION if X86_64 && TRANSPARENT_HUGEPAGE
        select ARCH_HAS_ACPI_TABLE_UPGRADE      if ACPI
        select ARCH_HAS_CACHE_LINE_SIZE
+       select ARCH_HAS_CURRENT_STACK_POINTER
        select ARCH_HAS_DEBUG_VIRTUAL
        select ARCH_HAS_DEBUG_VM_PGTABLE        if !X86_PAE
        select ARCH_HAS_DEVMEM_IS_ALLOWED
index 3326ee3903f33078143607ba0a67d131d567a03f..c349599601f82dee7ef2e913fb08fe52aa445a5f 100644 (file)
@@ -744,6 +744,15 @@ config IDLE_PAGE_TRACKING
 config ARCH_HAS_CACHE_LINE_SIZE
        bool
 
+config ARCH_HAS_CURRENT_STACK_POINTER
+       bool
+       help
+         In support of HARDENED_USERCOPY performing stack variable lifetime
+         checking, an architecture-agnostic way to find the stack pointer
+         is needed. Once an architecture defines an unsigned long global
+         register alias named "current_stack_pointer", this config can be
+         selected.
+
 config ARCH_HAS_PTE_DEVMAP
        bool
 
index d0d268135d96d6494949bb5eec702f7365ddc86a..5d34c40c16c2233e2d588b57058bfdb1b725d60e 100644 (file)
@@ -29,7 +29,7 @@
  * 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)
@@ -55,6 +55,17 @@ static noinline int check_stack_object(const void *obj, unsigned long len)
        if (ret)
                return ret;
 
+       /* Finally, check stack depth if possible. */
+#ifdef CONFIG_ARCH_HAS_CURRENT_STACK_POINTER
+       if (IS_ENABLED(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;
 }
 
@@ -280,7 +291,15 @@ 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,
+#ifdef CONFIG_ARCH_HAS_CURRENT_STACK_POINTER
+                       IS_ENABLED(CONFIG_STACK_GROWSUP) ?
+                               ptr - (void *)current_stack_pointer :
+                               (void *)current_stack_pointer - ptr,
+#else
+                       0,
+#endif
+                       n);
        }
 
        /* Check for bad heap object. */