linux-kernel - Re: [PATCH 4/6] x86,fpu: use disable_task_lazy_fpu

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:	Tue, 03 Feb 2015 17:01:21 -0500
From:	Rik van Riel <riel@...hat.com>
To:	Oleg Nesterov <oleg@...hat.com>
CC:	dave.hansen@...ux.intel.com, sbsiddha@...il.com,
	luto@...capital.net, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, fenghua.yu@...el.com, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] x86,fpu: use disable_task_lazy_fpu_restore helper

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/03/2015 02:08 PM, Oleg Nesterov wrote:
> On 02/02, Rik van Riel wrote:
>> 
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> On 02/02/2015 02:21 PM, Oleg Nesterov wrote:
>>> I'll try to read this patch tomorrow. Too late for me.
>>> 
>>> I think it is fine, but
>>> 
>>> On 02/02, riel@...hat.com wrote:
>>>> 
>>>> This also fixes the lazy FPU restore disabling in drop_fpu, 
>>>> which only really works when !use_eager_fpu(). ...
>>>> 
>>>> --- a/arch/x86/include/asm/fpu-internal.h +++ 
>>>> b/arch/x86/include/asm/fpu-internal.h @@ -396,7 +396,7 @@
>>>> static inline void drop_fpu(struct task_struct *tsk) *
>>>> Forget coprocessor state.. */ preempt_disable(); - 
>>>> tsk->thread.fpu_counter = 0; + 
>>>> task_disable_lazy_fpu_restore(tsk); __drop_fpu(tsk); 
>>>> clear_used_math();
>>> 
>>> perhaps this makes sense anyway, but I am not sure if the
>>> changelog is right.
>>> 
>>> Note that we clear PF_USED_MATH, last_fpu/has_fpu have no
>>> effect after this.
>> 
>> There are several code paths, including signal handler stack
>> setup and teardown, where we first clear PF_USED_MATH, but later
>> on set it again, after setting up a new math state for the task.
>> 
>> We need to ensure we always use that new math state, and never
>> lazily re-use what is still in the FPU registers.
> 
> Still can't understand....
> 
> I can only see only on example of re-setting PF_USED_MATH if
> eager_fpu, __restore_xstate_sig() does drop_fpu() +
> math_state_restore(). And this case looks fine in this sense...
> 
> 
> 
> Although let me repeat that imho math_state_restore() and all
> current users (including flush_thread() added by "don't abuse FPU
> in kernel threads") need cleanups in any case. I was going to (try
> to) do this if/when that series is applied. In particular, in this
> case math_state_restore() will wrongly return with irqs disabled or
> I am totally confused.
> 
> And set_used_math() if copy_from_user() fails doesn't look right,
> but this is another story.
> 
> 
> If I missed something, perhaps you can update the changelog to
> explain how exactly it fixes drop_fpu? Otherwise, imo this patch
> should not change it. As we already discussed, we can probably
> cleanup this disable_lazy_fpu logic, but this needs another
> change/discussion.

I don't think this changes the behaviour of any code path as the
kernel currently stands.

However, having drop_fpu() disable lazy restore in a more explicit
way may help us going forward, with the "delay FPU state restore to
kernel -> user space switch" series.

- -- 
All rights reversed
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJU0UUxAAoJEM553pKExN6DzSMIAIuPNXZ8P5zHAPMLePQhfaZK
QaatOplJ5F8GNN3bT2sJHLIPfDJVokMbGFv7ff8/jPsZ03uBm6onFPp/4Qn7dGZ4
FQyFX7OkmR7D4NJ0KZ9J1ghWhfRhmxfAqr/qwrdovUJ/Hfz73FdqGPYpP90qvY/2
0psSaW6ubJen6l9QYBear5juhQE3+akfwc/D1ItpZtZRUHOJTewBiww/9I/kUrDZ
mp1j4szVEjaQ9jB5Et4KO4EFaaEFzdj2JXwSV10neBgLrZ5dixYII1kUFQQFxvTd
bse2Rjsh0+6ESkUOoXLy1y0IYbjrQqEc+YdrGbnf64XJLIjj1a8U8F3dU5xPdCM=
=0+mw
-----END PGP SIGNATURE-----
--
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/