leandrof/linux
1
0
Fork 0
forked from Minki/linux
Code Issues Pull Requests Packages Projects Releases Wiki Activity

stackleak: rework poison scanning

Browse Source 
Currently we over-estimate the region of stack which must be erased.

To determine the region to be erased, we scan downwards for a contiguous
block of poison values (or the low bound of the stack). There are a few
minor problems with this today:

* When we find a block of poison values, we include this block within
  the region to erase.

  As this is included within the region to erase, this causes us to
  redundantly overwrite 'STACKLEAK_SEARCH_DEPTH' (128) bytes with
  poison.

* As the loop condition checks 'poison_count <= depth', it will run an
  additional iteration after finding the contiguous block of poison,
  decrementing 'erase_low' once more than necessary.

  As this is included within the region to erase, this causes us to
  redundantly overwrite an additional unsigned long with poison.

* As we always decrement 'erase_low' after checking an element on the
  stack, we always include the element below this within the region to
  erase.

  As this is included within the region to erase, this causes us to
  redundantly overwrite an additional unsigned long with poison.

  Note that this is not a functional problem. As the loop condition
  checks 'erase_low > task_stack_low', we'll never clobber the
  STACK_END_MAGIC. As we always decrement 'erase_low' after this, we'll
  never fail to erase the element immediately above the STACK_END_MAGIC.

In total, this can cause us to erase `128 + 2 * sizeof(unsigned long)`
bytes more than necessary, which is unfortunate.

This patch reworks the logic to find the address immediately above the
poisoned region, by finding the lowest non-poisoned address. This is
factored into a stackleak_find_top_of_poison() helper both for clarity
and so that this can be shared with the LKDTM test in subsequent
patches.

Signed-off-by: Mark 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: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20220427173128.2603085-8-mark.rutland@arm.com
This commit is contained in:
Mark Rutland 2022-04-27 18:31:22 +01:00 committed by Kees Cook
parent 0cfa2ccd28
commit 77cf2b6dee
2 changed files with 29 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
include/linux/stackleak.h
View File

@ -42,6 +42,32 @@ stackleak_task_high_bound(const struct task_struct *tsk)
return (unsigned long)task_pt_regs(tsk);
}
/*
* Find the address immediately above the poisoned region of the stack, where
* that region falls between 'low' (inclusive) and 'high' (exclusive).
*/
static __always_inline unsigned long
stackleak_find_top_of_poison(const unsigned long low, const unsigned long high)
{
const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long);
unsigned int poison_count = 0;
unsigned long poison_high = high;
unsigned long sp = high;
while (sp > low && poison_count < depth) {
sp -= sizeof(unsigned long);
if (*(unsigned long *)sp == STACKLEAK_POISON) {
poison_count++;
} else {
poison_count = 0;
poison_high = sp;
}
}
return poison_high;
}
static inline void stackleak_task_init(struct task_struct *t)
{
t->lowest_stack = stackleak_task_low_bound(t);

16
kernel/stackleak.c
View File

@ -74,20 +74,10 @@ static __always_inline void __stackleak_erase(void)
{
const unsigned long task_stack_low = stackleak_task_low_bound(current);
const unsigned long task_stack_high = stackleak_task_high_bound(current);
unsigned long erase_low = current->lowest_stack;
unsigned long erase_high;
unsigned int poison_count = 0;
const unsigned int depth = STACKLEAK_SEARCH_DEPTH / sizeof(unsigned long);
unsigned long erase_low, erase_high;
/* Search for the poison value in the kernel stack */
while (erase_low > task_stack_low && poison_count <= depth) {
if (*(unsigned long *)erase_low == STACKLEAK_POISON)
poison_count++;
else
poison_count = 0;
erase_low -= sizeof(unsigned long);
}
erase_low = stackleak_find_top_of_poison(task_stack_low,
current->lowest_stack);
#ifdef CONFIG_STACKLEAK_METRICS
current->prev_lowest_stack = erase_low;