• Mark Rutland's avatar
    lkdtm/stackleak: rework boundary management · 72b61896
    Mark Rutland authored
    There are a few problems with the way the LKDTM STACKLEAK_ERASING test
    manipulates the stack pointer and boundary values:
    
    * It uses the address of a local variable to determine the current stack
      pointer, rather than using current_stack_pointer directly. As the
      local variable could be placed anywhere within the stack frame, this
      can be an over-estimate of the true stack pointer value.
    
    * Is uses an estimate of the current stack pointer as the upper boundary
      when scanning for poison, even though prior functions could have used
      more stack (and may have updated current->lowest stack accordingly).
    
    * A pr_info() call is made in the middle of the test. As the printk()
      code is out-of-line and will make use of the stack, this could clobber
      poison and/or adjust current->lowest_stack. It would be better to log
      the metadata after the body of the test to avoid such problems.
    
    These have been observed to result in spurious test failures on arm64.
    
    In addition to this there are a couple of things which are sub-optimal:
    
    * To avoid the STACK_END_MAGIC value, it conditionally modifies 'left'
      if this contains more than a single element, when it could instead
      calculate the bound unconditionally using stackleak_task_low_bound().
    
    * It open-codes the poison scanning. It would be better if this used the
      same helper code as used by erasing function so that the two cannot
      diverge.
    
    This patch reworks the test to avoid these issues, making use of the
    recently introduced helpers to ensure this is aligned with the regular
    stackleak code.
    
    As the new code tests stack boundaries before accessing the stack, there
    is no need to fail early when the tracked or untracked portions of the
    stack extend all the way to the low stack boundary.
    
    As stackleak_find_top_of_poison() is now used to find the top of the
    poisoned region of the stack, the subsequent poison checking starts at
    this boundary and verifies that stackleak_find_top_of_poison() is
    working correctly.
    
    The pr_info() which logged the untracked portion of stack is now moved
    to the end of the function, and logs the size of all the portions of the
    stack relevant to the test, including the portions at the top and bottom
    of the stack which are not erased or scanned, and the current / lowest
    recorded stack usage.
    
    Tested on x86_64:
    
    | # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
    | lkdtm: Performing direct entry STACKLEAK_ERASING
    | lkdtm: stackleak stack usage:
    |   high offset: 168 bytes
    |   current:     336 bytes
    |   lowest:      656 bytes
    |   tracked:     656 bytes
    |   untracked:   400 bytes
    |   poisoned:    15152 bytes
    |   low offset:  8 bytes
    | lkdtm: OK: the rest of the thread stack is properly erased
    
    Tested on arm64:
    
    | # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
    | lkdtm: Performing direct entry STACKLEAK_ERASING
    | lkdtm: stackleak stack usage:
    |   high offset: 336 bytes
    |   current:     656 bytes
    |   lowest:      1232 bytes
    |   tracked:     1232 bytes
    |   untracked:   672 bytes
    |   poisoned:    14136 bytes
    |   low offset:  8 bytes
    | lkdtm: OK: the rest of the thread stack is properly erased
    
    Tested on arm64 with deliberate breakage to the starting stack value and
    poison scanning:
    
    | # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT
    | lkdtm: Performing direct entry STACKLEAK_ERASING
    | lkdtm: FAIL: non-poison value 24 bytes below poison boundary: 0x0
    | lkdtm: FAIL: non-poison value 32 bytes below poison boundary: 0xffff8000083dbc00
    ...
    | lkdtm: FAIL: non-poison value 1912 bytes below poison boundary: 0x78b4b9999e8cb15
    | lkdtm: FAIL: non-poison value 1920 bytes below poison boundary: 0xffff8000083db400
    | lkdtm: stackleak stack usage:
    |   high offset: 336 bytes
    |   current:     688 bytes
    |   lowest:      1232 bytes
    |   tracked:     576 bytes
    |   untracked:   288 bytes
    |   poisoned:    15176 bytes
    |   low offset:  8 bytes
    | lkdtm: FAIL: the thread stack is NOT properly erased!
    | lkdtm: Unexpected! This kernel (5.18.0-rc1-00013-g1f7b1f1e29e0-dirty aarch64) was built with CONFIG_GCC_PLUGIN_STACKLEAK=y
    Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
    Cc: Alexander Popov <alex.popov@linux.com>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Andy Lutomirski <luto@kernel.org>
    Cc: Kees Cook <keescook@chromium.org>
    Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    Link: https://lore.kernel.org/r/20220427173128.2603085-10-mark.rutland@arm.com
    72b61896
stackleak.c 2.76 KB