Re: [PATCH RFT v5 4/7] fork: Add shadow stack support to clone3()

Edgecombe, Rick P
2024-02-09T20:18:16Z

On Sat, 2024-02-03 at 00:05 +0000, Mark Brown wrote:
> +static bool shstk_consume_token(struct task_struct *tsk,
> +                               unsigned long addr)
> +{
> +       /*
> +        * SSP is aligned, so reserved bits and mode bit are a zero,
> just mark
> +        * the token 64-bit.
> +        */
> +       u64 expected = (addr - SS_FRAME_SIZE) | BIT(0);
> +       u64 val;
> +
> +       /* This should really be an atomic cpmxchg.  It is not. */
> +       __get_user(val, (__user u64 *)addr);
> +       if (val != expected)
> +               return false;
> +
> +       if (write_user_shstk_64((u64 __user *)addr, 0))
> +               return false;
> +
> +       return true;
> +}

So, don't we want to consume the token on the *new* task's MM, which
was already duplicated but still unmapped? In which case I think the
other arch's would need to GUP regardless of the existence of shadow
stack atomic ops.

If so, my question is, can we GUP on the new MM at this point? There is
a lot going in copy_process(). My first suspicion of complication is
the work on the child that happens in cgroup_post_fork().

I wonder about adding a shstk_post_fork() to make it easier to think
about and maintain, even if there are no issues today.