Messages in this thread | | | Date | Tue, 10 May 2022 20:05:58 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v2 05/13] stackleak: clarify variable names |
| |
On Tue, May 10, 2022 at 02:01:49PM +0100, Mark Rutland wrote: > On Sun, May 08, 2022 at 11:49:46PM +0300, Alexander Popov wrote: > > On 27.04.2022 20:31, Mark Rutland wrote: > > > The logic within __stackleak_erase() can be a little hard to follow, as > > > `boundary` switches from being the low bound to the high bound mid way > > > through the function, and `kstack_ptr` is used to represent the start of > > > the region to erase while `boundary` represents the end of the region to > > > erase. > > > > > > Make this a little clearer by consistently using clearer variable names. > > > The `boundary` variable is removed, the bounds of the region to erase > > > are described by `erase_low` and `erase_high`, and bounds of the task > > > stack are described by `task_stack_low` and `task_stck_high`. > > > > A typo here in `task_stck_high`. > > Ah; whoops.
No worries; I fixed this when I took the patch.
> > That was also the main reason why I reused the 'boundary' variable: I wanted > > the compiler to allocate it in the register and I avoided creating many > > local variables. > > > > Mark, did your refactoring make the compiler allocate local variables on the > > stack instead of the registers? > > Considering the whole series, testing with GCC 11.1.0: > > * On arm64: > before: stackleak_erase() uses 48 bytes of stack > after: stackleak_erase() uses 0 bytes of stack > > Note: this is entirely due to patch 1; arm64 has enough GPRs that it > doesn't need to use the stack. > > * On x86_64: > before: stackleak_erase() uses 0 bytes of stack > after: stackleak_erase() uses 0 bytes of stack > > * On i386 > before: stackleak_erase() uses 8 bytes of stach > after: stackleak_erase() uses 16 bytes of stack > > The i386 case isn't ideal, but given that those bytes will easily be used by > the entry triage code before getting to any syscall handling, I don't believe > that's an issue in practice.
I am biased and totally fine with choosing a solution where 64-bit improvement comes at a 32-bit cost.
-- Kees Cook
| |