linux-kernel - Re: [PATCH 2/5] x86, fpu: don't drop_fpu() in __restore_xstate_sig() if use_eager

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]

Date:	Mon, 25 Aug 2014 19:09:20 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Al Viro <viro@...iv.linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Fenghua Yu <fenghua.yu@...el.com>,
	Suresh Siddha <sbsiddha@...il.com>,
	Bean Anderson <bean@...lsystems.com>,
	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	the arch/x86 maintainers <x86@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/5] x86, fpu: don't drop_fpu() in
	__restore_xstate_sig() if use_eager_fpu()

On 08/25, Linus Torvalds wrote:
>
> On Mon, Aug 25, 2014 at 7:41 AM, Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > I think this should be safe, because this thread and/or swapper/0 can
> > do nothing with with fpu->state, and they should not use fpu.
>
> .. but if that's the case, then what was wrong with the old code

Confused... Just in case, I think that you mean current code, and ignoring
the lack of preempt_disable() around math_state_restore() it is correct.

I'd like to change it only because this code is the main source of the
nasty special case, used_math() and/or __thread_has_fpu(current) can be
false even if use_eager_fpu().

> that
> just copied the state over the unused space from the user space
> buffer?

But it is not unused? Although I probably misunderstood you from the
very beginning.

OK, what I meant that without switch_fpu_xstate(init_task.fpu.state)
or another hack we can't avoid drop_fpu() which leads to this special
case.

Currently __copy_from_user(&xstate->xsave) copies the new registers
right into this thread's fpu->state. If this thread is preempted before
math_state_restore(), the context switch (__save_init_fpu) will overwrite
the same buffer, the result of __copy_from_user() can be simply lost
(entirely or not).

With this patch we can safely do __copy_from_user(xstate), this buffer
is not used until the 2nd switch_fpu_xstate().

> You can't have it both ways. Either the old code was fine (because it
> doesn't use the buffer while it is in flux), or the new code is broken
> (because it uses the shared buffer). Your choice.No?

It uses the shared buffer, yes. But in this case (I think! please correct
me!), when this thread uses the swapper's fpu->state, schedule() ->
fpu_xsave() into this shared buffer should be fine because it should write
the same content?

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/