linux-kernel - Re: ptrace && fpu_lazy

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:	Sat, 14 Apr 2012 19:03:44 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	Chuck Ebbert <chuckebbert.lk@...il.com>,
	Jan Kratochvil <jan.kratochvil@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: ptrace && fpu_lazy_restore

On Sat, Apr 14, 2012 at 4:52 PM, Oleg Nesterov <oleg@...hat.com> wrote:
>
> But I am not sure about the fix, and in any case I need more time
> to read this new code.

Oh, the fix is obviously correct. Yes, when we write to the FPU state
area, we need to invalidate the cached copy in the FPU itself.

It used to be that you could never have cached FPU contents unless you
owned the CPU (well, not strictly true - we used to do *really* lazy
FPU save/restone long long long ago in the pre-SMP days too), so this
bug is new with the lazy restore.

The only part I don't like about it is how hacky the location is. It
turns out that if the process *itself* does a "init_fpu()" call, then
the call to "unlazy_fpu()" will do the right thing and invalidate the
FPU state by setting fpu_owner_task to zero. But when we're ptracing
and doing an "init_fpu()" for somebody else, we'll skip that all,
because you can only really unlazy_fpu() yourself.

But that means that "init_fpu()" acts subtly differently depending on
whether you call it on yourself, or on another process.

So I actually think that I would prefer the patch that invalidates the
FPU caches more aggressively. Sure, we don't really *need* to
invalidate if we're just reading, but I'd almost prefer to just have
it done once in "init_fpu()".

The only case where we care about the FPU caches remaining is actually
the nice normal "we just switched tasks through normal scheduling".
Any path that calls "init_fpu()" is special enough that I think we
should just make sure the FPU state is all in memory. Hmm?

So I think I'd prefer the attached (UNTESTED!) oneliner instead.

If that works fine for people, how about sending it back to me with a
commit log and sign-off? I don't want to get credit for this patch
that is just a "let's do it in a different place" version of yours.

                             Linus

Download attachment "patch.diff" of type "application/octet-stream" (421 bytes)