linux-kernel - Re: [PATCH 3/3, v4] x86/fpu: Remove init_task FPU state dependencies, add debugging warning for PF

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: Fri, 7 Jun 2024 12:56:49 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, Andy Lutomirski <luto@...capital.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Dave Hansen <dave@...1.net>, Peter Zijlstra <peterz@...radead.org>,
	Borislav Petkov <bp@...en8.de>, "H . Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Uros Bizjak <ubizjak@...il.com>
Subject: Re: [PATCH 3/3, v4] x86/fpu: Remove init_task FPU state
 dependencies, add debugging warning for PF_KTHREAD tasks


* Oleg Nesterov <oleg@...hat.com> wrote:

> On 06/06, Ingo Molnar wrote:
> >
> > I changed the debug check to test for PF_KTHREAD, and to return NULL:
> >
> > +#ifdef CONFIG_X86_DEBUG_FPU
> > +struct fpu *x86_task_fpu(struct task_struct *task)
> > +{
> > +	if (WARN_ON_ONCE(task->flags & PF_KTHREAD))
> > +		return NULL;
> > +
> > +	return (void *)task + sizeof(*task);
> > +}
> > +#endif
> 
> How many users enable CONFIG_X86_DEBUG_FPU?

Ubuntu does:

  kepler:~/tip> grep X86_DEBUG_FPU /boot/config-6.*
  /boot/config-6.8.0-35-generic:CONFIG_X86_DEBUG_FPU=y

Fedora doesn't:

  kepler:~/s/tmp> grep X86_DEBUG_FPU ./lib/modules/6.9.0-64.fc41.x86_64/config
  # CONFIG_X86_DEBUG_FPU is not set

So it's a bit of a hit and miss, but at least one major distribution does, 
which is all that we need really.

> [...] Perhaps it makes sense to check PF_KTHREAD unconditionally for the 
> start, them add if (IS_ENABLED(X86_DEBUG_FPU)). But I won't insist.

I think Ubuntu ought to add enough debug coverage - and this check isn't 
exactly cheap, given how it replaces a simple build time structure offset 
with a full-blown function call ...

> For the record, I think we can later change this code to check
> 
> 	task->flags & (PF_KTHREAD | PF_USER_WORKER)
> 
> but I guess this needs some (simple) changes in the ptrace/coredump
> paths.

Sounds useful, would you be interested in cooking up a series on top of 
tip:master (or tip:WIP.x86/fpu), if you are interested and have the time? 

I'll send out one last iteration of this series today, otherwise the 
changes seem close to final for an upstream merge via the tip:x86/fpu tree.

Thanks,

	Ingo