linux-kernel - Re: [PATCH 2/4] arch/x86: implement the process_vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YN7TgV9RDJTRaY8R@hirez.programming.kicks-ass.net>
Date:   Fri, 2 Jul 2021 10:51:13 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Andrei Vagin <avagin@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
        linux-um@...ts.infradead.org, criu@...nvz.org, avagin@...gle.com,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Anton Ivanov <anton.ivanov@...bridgegreys.com>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Ingo Molnar <mingo@...hat.com>, Jeff Dike <jdike@...toit.com>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Oleg Nesterov <oleg@...hat.com>,
        Richard Weinberger <richard@....at>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 2/4] arch/x86: implement the process_vm_exec syscall


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;
> +}