lkml.org 
[lkml]   [2021]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v28 25/32] x86/cet/shstk: Handle thread shadow stack
From
Date
On 7/22/21 1:52 PM, Yu-cheng Yu wrote:
> + if (!stack_size)
> + stack_size = min_t(unsigned long long, rlimit(RLIMIT_STACK), SZ_4G);
> +
> + if (!shstk->size)
> + return 0;
> +
> + /*
> + * For CLONE_VM, except vfork, the child needs a separate shadow
> + * stack.
> + */
> + if ((clone_flags & (CLONE_VFORK | CLONE_VM)) != CLONE_VM)
> + return 0;
> +
> + /*
> + * This is in clone() syscall and fpu__copy() already copies xstates
> + * from the parent. If get_xsave_addr() returns null, then XFEATURE_
> + * CET_USER is still in init state, which certainly is an error.
> + */
> + state = get_xsave_addr(&tsk->thread.fpu.state.xsave, XFEATURE_CET_USER);
> + if (!state)
> + return -EINVAL;

I don't care much for that comment.

This code is meant to copy shadow stack config information into children
when it is already enabled. We *just* checked for that above in the
"shstk->size" check. The fact that this is called from clone() is
irrelevant. The shadow stack enabling status *is*.

I think I'd rather this be more along the lines of:

/*
* 'tsk' is configured with a shadow stack and the fpu.state is
* up to date since it was just copied from the parent. There
* must be a valid non-init CET state location in the buffer.
*/

There is also a strong enough assumption violation that I'd probably
WARN() in addition to returning -EINVAL.

\
 
 \ /
  Last update: 2021-07-22 23:06    [W:1.370 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site