Messages in this thread | | | From | Mark Rutland <> | Subject | [PATCH v2 00/13] stackleak: fixes and rework | Date | Wed, 27 Apr 2022 18:31:15 +0100 |
| |
Hi Alexander, Kees,
This is the vs I promised. Since Alexander wanted to look at this in more detail (and since this is subtle and needs review), I'm assuming that Kees will pick this up some time next week after that's happened, if all goes well. :)
This series reworks the stackleak code and the associated LKDTM test. The first patch fixes some latent issues on arm64, and the subsequent patches improve the code to improve clarity and permit better code generation. Patches 8-10 address some latent issues in the LKDTM test and add more diagnostic output.
Since v1 [1]: * Fix and rework the LKDTM test * Rework the poison scan
[1] https://lore.kernel.org/lkml/20220425115603.781311-1-mark.rutland@arm.com/
Benchmarking arm64 with a QEMU HVF VM on an M1 Macbook Pro shows a reasonable improvement when stackleak is enabled. I've included figures for when stackleak is dynamically disabled with v2:
* Calling getpid 1^22 times in a loop (avg 50 runs)
5.18-rc1/on: 0.652099387s ( +- 0.13% ) v1/on: 0.641005661s ( +- 0.13% ) ; 1.7% improvement v2/on: 0.611699824s ( +- 0.08% ) ; 6.2% improvement v2/off: 0.507505632s ( +- 0.15% )
* perf bench sched pipe (single run)
5.18-rc1/on: 2.138s total v1/on: 2.118s total ; 0.93% improvement v2/on: 2.116s total ; 1.03% improvement v2/off: 1.708s total
The large jump between v1 and v2 is largely due to the changes to poison scanning avoiding redundantly overwriting poison. For the getpid syscall the poison which gets overwritten is a substantial fraction of the stack usage, and for more involved syscalls this may be a trivial fraction, so the average benefit is fairly small.
With the whole series applied, the LKDTM test reliably passes on both arm64 and x86_64, e.g.
| # uname -a | Linux buildroot 5.18.0-rc1-00013-g26f638ab0d7c #3 SMP PREEMPT Wed Apr 27 16:21:37 BST 2022 aarch64 GNU/Linux | # echo STACKLEAK_ERASING > /sys/kernel/debug/provoke-crash/DIRECT | lkdtm: Performing direct entry STACKLEAK_ERASING | lkdtm: stackleak stack usage: | high offset: 336 bytes | current: 688 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
The first patch fixes the major issues on arm64, and is Cc'd to stable for backporting.
The second patch is a trivial optimization for when stackleak is dynamically disabled.
The subsequent patches rework the way stackleak manipulates the stack boundary values. This is partially for clarity (e.g. with separate 'low' and 'high' boundary variables), and also permits the compiler to generate more optimal assembly by generating the high and low bounds from the same base.
Patch 6 changes the way that `current->lowest_stack` is reset prior to return to userspace. The existing code uses an undocumented offset relative to the top of the stack which doesn't make much sense (as thie sometimes falls within the task's pt_regs, or sometimes adds 600+ bytes to erase upon the next exit to userspace). For now I've removed the offset entirely.
Patch 7 changes the way we scan for poison. This fixes what I believe are fencepost errors, and also avoids the need to redundantly overwrite poison.
Patches 8-11 rework the LKDTM test, fixing cases where the test could spuriously fail and improving the diagnostic output.
Patches 12 and 13 add stackleak_erase_on_task_stack() and stackleak_erase_off_task_stack() that can be used when a caller knows they're always on or off the task stack respectively, avoiding redundant logic to check this and generate the high boundary value. The former is used on arm64 where we always perform the erase on the task stack, and I've included the latter for completeness (as e.g. it could be used on x86) given it is trivial.
Thanks, Mark.
Mark Rutland (13): arm64: stackleak: fix current_top_of_stack() stackleak: move skip_erasing() check earlier stackleak: remove redundant check stackleak: rework stack low bound handling stackleak: clarify variable names stackleak: rework stack high bound handling stackleak: rework poison scanning lkdtm/stackleak: avoid spurious failure lkdtm/stackleak: rework boundary management lkdtm/stackleak: prevent unexpected stack usage lkdtm/stackleak: check stack boundaries stackleak: add on/off stack variants arm64: entry: use stackleak_erase_on_task_stack()
arch/arm64/include/asm/processor.h | 10 +-- arch/arm64/kernel/entry.S | 2 +- drivers/misc/lkdtm/stackleak.c | 133 +++++++++++++++++++---------- include/linux/stackleak.h | 55 +++++++++++- kernel/stackleak.c | 105 ++++++++++++++--------- 5 files changed, 210 insertions(+), 95 deletions(-)
-- 2.30.2
| |