[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <57dcc827-052a-94cd-31d4-286675f9d506@intel.com>
Date: Wed, 10 Feb 2021 13:38:10 -0800
From: "Yu, Yu-cheng" <yu-cheng.yu@...el.com>
To: Kees Cook <keescook@...omium.org>
Cc: x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, linux-mm@...ck.org,
linux-arch@...r.kernel.org, linux-api@...r.kernel.org,
Arnd Bergmann <arnd@...db.de>,
Andy Lutomirski <luto@...nel.org>,
Balbir Singh <bsingharora@...il.com>,
Borislav Petkov <bp@...en8.de>,
Cyrill Gorcunov <gorcunov@...il.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Eugene Syromiatnikov <esyr@...hat.com>,
Florian Weimer <fweimer@...hat.com>,
"H.J. Lu" <hjl.tools@...il.com>, Jann Horn <jannh@...gle.com>,
Jonathan Corbet <corbet@....net>,
Mike Kravetz <mike.kravetz@...cle.com>,
Nadav Amit <nadav.amit@...il.com>,
Oleg Nesterov <oleg@...hat.com>, Pavel Machek <pavel@....cz>,
Peter Zijlstra <peterz@...radead.org>,
Randy Dunlap <rdunlap@...radead.org>,
"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
Vedvyas Shanbhogue <vedvyas.shanbhogue@...el.com>,
Dave Martin <Dave.Martin@....com>,
Weijiang Yang <weijiang.yang@...el.com>,
Pengfei Xu <pengfei.xu@...el.com>, haitao.huang@...el.com
Subject: Re: [PATCH v20 21/25] x86/cet/shstk: Handle signals for shadow stack
On 2/10/2021 11:58 AM, Kees Cook wrote:
> On Wed, Feb 10, 2021 at 09:56:59AM -0800, Yu-cheng Yu wrote:
>> To deliver a signal, create a shadow stack restore token and put the token
>> and the signal restorer address on the shadow stack. For sigreturn, verify
>> the token and restore from it the shadow stack pointer.
>>
>> A shadow stack restore token marks a restore point of the shadow stack.
>> The token is distinctively different from any shadow stack address.
>
> How is it different? It seems like it just has the last 2 bits
> masked/set?
>
For example, for 64-bit apps,
A shadow stack pointer value (*ssp) has to be in some code area, but for
a token, (*ptr_of_token) = (ptr_of_token + 8), which has to be within
the same shadow stack area. In cet_verify_rstor_token(), this is checked.
>> In sigreturn, restoring from a token ensures the target address is the
>> location pointed by the token.
>
> As in, a token (real stack address with 2-bit mask) is checked against
> the real stack address? I don't see a comparison -- it only checks that
> it is < TASK_SIZE.
>
> How does cet_restore_signal() figure into this? (As in, the MSR writes?)
>
The kernel takes the restore address from the token. It will not
mistakenly take a wrong address from the shadow stack. I will put this
in my commit logs.
[...]
>> Introduce WRUSS, which is a kernel-mode instruction but writes directly to
>> user shadow stack. It is used to construct the user signal stack as
>> described above.
>>
>> Currently there is no systematic facility for extending a signal context.
>> Introduce a signal context extension 'struct sc_ext', which is used to save
>> shadow stack restore token address and WAIT_ENDBR status. WAIT_ENDBR will
>> be introduced later in the Indirect Branch Tracking (IBT) series, but add
>> that into sc_ext now to keep the struct stable in case the IBT series is
>> applied later.
>>
>> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@...el.com>
[...]
>> diff --git a/arch/x86/kernel/cet.c b/arch/x86/kernel/cet.c
>> index d25a03215984..08e43d9b5176 100644
>> --- a/arch/x86/kernel/cet.c
>> +++ b/arch/x86/kernel/cet.c
>> @@ -19,6 +19,8 @@
>> #include <asm/fpu/xstate.h>
>> #include <asm/fpu/types.h>
>> #include <asm/cet.h>
>> +#include <asm/special_insns.h>
>> +#include <uapi/asm/sigcontext.h>
>>
>> static void start_update_msrs(void)
>> {
>> @@ -72,6 +74,80 @@ static unsigned long alloc_shstk(unsigned long size, int flags)
>> return addr;
>> }
>>
>> +#define TOKEN_MODE_MASK 3UL
>> +#define TOKEN_MODE_64 1UL
>> +#define IS_TOKEN_64(token) (((token) & TOKEN_MODE_MASK) == TOKEN_MODE_64)
>> +#define IS_TOKEN_32(token) (((token) & TOKEN_MODE_MASK) == 0)
>> +
>> +/*
>> + * Verify the restore token at the address of 'ssp' is
>> + * valid and then set shadow stack pointer according to the
>> + * token.
>> + */
>> +int cet_verify_rstor_token(bool ia32, unsigned long ssp,
>> + unsigned long *new_ssp)
>> +{
>> + unsigned long token;
>> +
>> + *new_ssp = 0;
>> +
>> + if (!IS_ALIGNED(ssp, 8))
>> + return -EINVAL;
>> +
>> + if (get_user(token, (unsigned long __user *)ssp))
>> + return -EFAULT;
>> +
>> + /* Is 64-bit mode flag correct? */
>> + if (!ia32 && !IS_TOKEN_64(token))
>> + return -EINVAL;
>> + else if (ia32 && !IS_TOKEN_32(token))
>> + return -EINVAL;
>> +
>> + token &= ~TOKEN_MODE_MASK;
>> +
>> + /*
>> + * Restore address properly aligned?
>> + */
>> + if ((!ia32 && !IS_ALIGNED(token, 8)) || !IS_ALIGNED(token, 4))
>> + return -EINVAL;
>> +
>> + /*
>> + * Token was placed properly?
>> + */
>> + if (((ALIGN_DOWN(token, 8) - 8) != ssp) || token >= TASK_SIZE_MAX)
>> + return -EINVAL;
>> +
>> + *new_ssp = token;
>> + return 0;
>> +}
>> +
>> +/*
>> + * Create a restore token on the shadow stack.
>> + * A token is always 8-byte and aligned to 8.
>> + */
>> +static int create_rstor_token(bool ia32, unsigned long ssp,
>> + unsigned long *new_ssp)
>> +{
>> + unsigned long addr;
>> +
>> + *new_ssp = 0;
>> +
>> + if ((!ia32 && !IS_ALIGNED(ssp, 8)) || !IS_ALIGNED(ssp, 4))
>> + return -EINVAL;
>> +
>> + addr = ALIGN_DOWN(ssp, 8) - 8;
>> +
>> + /* Is the token for 64-bit? */
>> + if (!ia32)
>> + ssp |= TOKEN_MODE_64;
>> +
>> + if (write_user_shstk_64(addr, ssp))
>> + return -EFAULT;
>> +
>> + *new_ssp = addr;
>> + return 0;
>> +}
>> +
[...]
Powered by blists - more mailing lists