Re: [RFC PATCH v4 20/27] x86/cet/shstk: Signal handling for shadow stack

On Fri, Sep 21, 2018 at 5:09 PM Yu-cheng Yu [off-list ref] wrote:

When setting up a signal, the kernel creates a shadow stack
restore token at the current SHSTK address and then stores the
token's address in the signal frame, right after the FPU state.
Before restoring a signal, the kernel verifies and then uses the
restore token to set the SHSTK pointer.

[...]

+#ifdef CONFIG_X86_64
+static int copy_ext_from_user(struct sc_ext *ext, void __user *fpu)
+{
+       void __user *p;
+
+       if (!fpu)
+               return -EINVAL;
+
+       p = fpu + fpu_user_xstate_size + FP_XSTATE_MAGIC2_SIZE;
+       p = (void __user *)ALIGN((unsigned long)p, 8);
+
+       if (!access_ok(VERIFY_READ, p, sizeof(*ext)))
+               return -EFAULT;
+
+       if (__copy_from_user(ext, p, sizeof(*ext)))
+               return -EFAULT;

Why do you first manually call access_ok(), then call
__copy_from_user() with the same size? Just use "if
(copy_from_user(ext, p, sizeof(*ext)))" (without underscores) and get
rid of the access_ok().

+       if (ext->total_size != sizeof(*ext))
+               return -EINVAL;
+       return 0;
+}
+
+static int copy_ext_to_user(void __user *fpu, struct sc_ext *ext)
+{
+       void __user *p;
+
+       if (!fpu)
+               return -EINVAL;
+
+       if (ext->total_size != sizeof(*ext))
+               return -EINVAL;
+
+       p = fpu + fpu_user_xstate_size + FP_XSTATE_MAGIC2_SIZE;
+       p = (void __user *)ALIGN((unsigned long)p, 8);
+
+       if (!access_ok(VERIFY_WRITE, p, sizeof(*ext)))
+               return -EFAULT;
+
+       if (__copy_to_user(p, ext, sizeof(*ext)))
+               return -EFAULT;

Same as above.

+       return 0;
+}