Re: [PATCH 2/4] arch/x86: implement the process_vm_exec syscall

[Peter Zijlstra <peterz@infradead.org>]

I'm terrified of all of this...

On Tue, Apr 13, 2021 at 10:52:15PM -0700, Andrei Vagin wrote:

> +long swap_vm_exec_context(struct sigcontext __user *uctx)
> +{
> +	struct sigcontext ctx = {};
> +	sigset_t set = {};
> +
> +
> +	if (copy_from_user(&ctx, uctx, CONTEXT_COPY_SIZE))
> +		return -EFAULT;
> +	/* A floating point state is managed from user-space. */
> +	if (ctx.fpstate != 0)
> +		return -EINVAL;
> +	if (!user_access_begin(uctx, sizeof(*uctx)))
> +		return -EFAULT;
> +	unsafe_put_sigcontext(uctx, NULL, current_pt_regs(), (&set), Efault);
> +	user_access_end();

But here you save the sigcontext without FPU state.

> +
> +	if (__restore_sigcontext(current_pt_regs(), &ctx, 0))
> +		goto badframe;

And here you restore sigcontext, *with* FPU state.  At which point your
FPU state is irrecoverably lost.

Also, I'm not at all convinced this can ever do the right thing when the
tasks don't agree on what the FPU state is. I suppose in the best case
the save will EFAULT.

> +
> +	return 0;
> +Efault:
> +	user_access_end();
> +badframe:
> +	signal_fault(current_pt_regs(), uctx, "swap_vm_exec_context");
> +	return -EFAULT;
> +}