From: Kees Cook <keescook@chromium.org>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linuxppc-dev@lists.ozlabs.org, Paul Mackerras <paulus@samba.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powerpc/32: Clear volatile regs on syscall exit
Date: Wed, 23 Feb 2022 11:34:08 -0800 [thread overview]
Message-ID: <202202231131.08B7EC1@keescook> (raw)
In-Reply-To: <28b040bd2357a1879df0ca1b74094323f778a472.1645636285.git.christophe.leroy@csgroup.eu>
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?
(And you may want to consider wiping all "unused" registers at syscall
entry as well.)
-Kees
> 1: lwz r2,GPR2(r1)
> lwz r1,GPR1(r1)
> rfi
> --
> 2.34.1
>
--
Kees Cook
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Michael Ellerman <mpe@ellerman.id.au>,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/32: Clear volatile regs on syscall exit
Date: Wed, 23 Feb 2022 11:34:08 -0800 [thread overview]
Message-ID: <202202231131.08B7EC1@keescook> (raw)
In-Reply-To: <28b040bd2357a1879df0ca1b74094323f778a472.1645636285.git.christophe.leroy@csgroup.eu>
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?
(And you may want to consider wiping all "unused" registers at syscall
entry as well.)
-Kees
> 1: lwz r2,GPR2(r1)
> lwz r1,GPR1(r1)
> rfi
> --
> 2.34.1
>
--
Kees Cook
next prev parent reply other threads:[~2022-02-23 19:34 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-23 17:11 [PATCH] powerpc/32: Clear volatile regs on syscall exit Christophe Leroy
2022-02-23 17:11 ` Christophe Leroy
2022-02-23 19:34 ` Kees Cook [this message]
2022-02-23 19:34 ` Kees Cook
2022-02-24 7:00 ` Christophe Leroy
2022-02-24 7:00 ` Christophe Leroy
2022-02-23 20:48 ` Gabriel Paubert
2022-02-23 20:48 ` Gabriel Paubert
2022-02-23 23:27 ` Segher Boessenkool
2022-02-23 23:27 ` Segher Boessenkool
2022-02-24 8:29 ` Gabriel Paubert
2022-02-24 8:29 ` Gabriel Paubert
2022-02-24 12:49 ` Segher Boessenkool
2022-02-24 12:49 ` Segher Boessenkool
2022-02-24 6:41 ` Christophe Leroy
2022-02-24 6:41 ` Christophe Leroy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202202231131.08B7EC1@keescook \
--to=keescook@chromium.org \
--cc=christophe.leroy@csgroup.eu \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.