lkml.org 
[lkml]   [2022]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] powerpc/32: Clear volatile regs on syscall exit
Date


Le 23/02/2022 à 20:34, Kees Cook a écrit :
> On Wed, Feb 23, 2022 at 06:11:36PM +0100, Christophe Leroy wrote:
>> Commit a82adfd5c7cb ("hardening: Introduce CONFIG_ZERO_CALL_USED_REGS")
>> added zeroing of used registers at function exit.
>>
>> At the time being, PPC64 clears volatile registers on syscall exit but
>> PPC32 doesn't do it for performance reason.
>>
>> Add that clearing in PPC32 syscall exit as well, but only when
>> CONFIG_ZERO_CALL_USED_REGS is selected.
>>
>> On an 8xx, the null_syscall selftest gives:
>> - Without CONFIG_ZERO_CALL_USED_REGS : 288 cycles
>> - With CONFIG_ZERO_CALL_USED_REGS : 305 cycles
>> - With CONFIG_ZERO_CALL_USED_REGS + this patch : 319 cycles
>>
>> Note that (independent of this patch), with pmac32_defconfig,
>> vmlinux size is as follows with/without CONFIG_ZERO_CALL_USED_REGS:
>>
>> text data bss dec hex filename
>> 9578869 2525210 194400 12298479 bba8ef vmlinux.without
>> 10318045 2525210 194400 13037655 c6f057 vmlinux.with
>>
>> That is a 7.7% increase on text size, 6.0% on overall size.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/kernel/entry_32.S | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index 7748c278d13c..199f23092c02 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -151,6 +151,21 @@ syscall_exit_finish:
>> bne 3f
>> mtcr r5
>>
>> +#ifdef CONFIG_ZERO_CALL_USED_REGS
>> + /* Zero volatile regs that may contain sensitive kernel data */
>> + li r0,0
>> + li r4,0
>> + li r5,0
>> + li r6,0
>> + li r7,0
>> + li r8,0
>> + li r9,0
>> + li r10,0
>> + li r11,0
>> + li r12,0
>> + mtctr r0
>> + mtxer r0
>> +#endif
>
> I think this should probably be unconditional -- if this is actually
> leaking kernel pointers (or data) that's pretty bad. :|
>
> If you really want to leave it build-time selectable, maybe add a new
> config that gets "select"ed by CONFIG_ZERO_CALL_USED_REGS?

You mean a CONFIG that is selected by CONFIG_ZERO_CALL_USED_REGS and may
also be selected by the user when CONFIG_ZERO_CALL_USED_REGS is not
selected ?

At exit:
- contain of r4 is loaded in LR
- contain of r5 is loaded in CR
- contain of r7 is were we branch after switching back to user mode
- contain of r8 is loaded in MSR. Allthough MSR can't be read by the
user, there is nothing secret in it.
- XER contains arithmetic flags, nothing really sensitive.

So remain r0, r6, r9 to r12 and ctr.

Maybe a compromise could be to only clear those when
CONFIG_ZERO_CALL_USED_REGS is not selected ?

>
> (And you may want to consider wiping all "unused" registers at syscall
> entry as well.)

How "unused" ?

At syscall entry we have syscall NR in r0, syscall args in r3 to r8.
The handler uses r9, r10, r11 and r12 prior to re-enabling MMU and
taking any conditional branche.
r1 and r2 are also soon set and used (r1 is stack ptr, r2 is ptr to
current task struct) and restored from stack at the end.
r13-r31 are callee saved/restored.

Christophe
\
 
 \ /
  Last update: 2022-02-24 08:01    [W:0.091 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site