linux-kernel - Re: [RFC][PATCH] x86, fpu: dynamically allocate 'struct 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] [day] [month] [year] [list]

Date:	Sat, 18 Jul 2015 05:40:24 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	"H. Peter Anvin" <hpa@...or.com>
Cc:	Dave Hansen <dave@...1.net>, tglx@...utronix.de, mingo@...hat.com,
	x86@...nel.org, peterz@...radead.org, bp@...en8.de,
	luto@...capital.net, torvalds@...ux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH] x86, fpu: dynamically allocate 'struct fpu'


* H. Peter Anvin <hpa@...or.com> wrote:

> On 07/16/2015 12:14 PM, Dave Hansen wrote:
> > The FPU rewrite removed the dynamic allocations of 'struct fpu'.
> > But, this potentially wastes massive amounts of memory (2k per
> > task on systems that do not have AVX-512 for instance).
> > 
> > Instead of having a separate slab, this patch just appends the
> > space that we need to the 'task_struct' which we dynamically
> > allocate already.  This saves from doing an extra slab allocation
> > at fork().  The only real downside here is that we have to stick
> > everything and the end of the task_struct.  But, I think the
> > BUILD_BUG_ON()s I stuck in there should keep that from being too
> > fragile.
> > 
> > This survives a quick build and boot in a VM.  Does anyone see any
> > real downsides to this?
> 
> No.  I have also long advocated for merging task_struct and thread_info into a 
> common structure and get it off the stack; it would improve security and avoid 
> weird corner cases in the irqstack handling.

Note that we have 3 related 'task state' data structures with overlapping purpose:

  task_struct
   thread_struct
  thread_info

where thread_struct is embedded in task_struct currently.

So to turn it all into a single structure we'd have to merge thread_info into 
thread_struct. thread_info was put on the kernel stack due to the ESP trick we 
played long ago - but that is moot these days.

So I think what we want is not some common structure, but to actually merge all of 
thread_info into thread_struct for arch details and into task_struct for generic 
fields, and only have:

  task_struct                /* generic fields */
   thread_struct             /* arch details */

this can be done gradually, field by field, and in the end thread_info can be 
eliminated altogether.

The only real complication is that it affects every architecture. The good news is 
that most of the thread_info layout details are wrapped in various constructs like 
test_ti_thread_flag() and task_thread_info().

While at it we might as well rename 'thread_struct' to 'arch_task_struct':

  task_struct                  /* generic fields */
  arch_task_struct             /* arch details */

to make it really clear and easy to understand at a glance - as the current naming 
is has become ambiguous and slightly confusing the moment we introduced threading.

Thanks,

	Ingo
--
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/