Skip to content

Commit cb7ca40

Browse files
author
Ingo Molnar
committed
x86/fpu: Make task_struct::thread constant size
Turn thread.fpu into a pointer. Since most FPU code internals work by passing around the FPU pointer already, the code generation impact is small. This allows us to remove the old kludge of task_struct being variable size: struct task_struct { ... /* * New fields for task_struct should be added above here, so that * they are included in the randomized portion of task_struct. */ randomized_struct_fields_end /* CPU-specific state of this task: */ struct thread_struct thread; /* * WARNING: on x86, 'thread_struct' contains a variable-sized * structure. It *MUST* be at the end of 'task_struct'. * * Do not put anything below here! */ }; ... which creates a number of problems, such as requiring thread_struct to be the last member of the struct - not allowing it to be struct-randomized, etc. But the primary motivation is to allow the decoupling of task_struct from hardware details (<asm/processor.h> in particular), and to eventually allow the per-task infrastructure: DECLARE_PER_TASK(type, name); ... per_task(current, name) = val; ... which requires task_struct to be a constant size struct. The fpu_thread_struct_whitelist() quirk to hardened usercopy can be removed, now that the FPU structure is not embedded in the task struct anymore, which reduces text footprint a bit. Fixed-by: Oleg Nesterov <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Cc: Andy Lutomirski <[email protected]> Cc: Brian Gerst <[email protected]> Cc: Chang S. Bae <[email protected]> Cc: H. Peter Anvin <[email protected]> Cc: Linus Torvalds <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent e3bfa38 commit cb7ca40

File tree

5 files changed

+36
-41
lines changed

5 files changed

+36
-41
lines changed

arch/x86/include/asm/processor.h

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -516,21 +516,19 @@ struct thread_struct {
516516
#endif
517517

518518
/* Floating point and extended processor state */
519-
struct fpu fpu;
520-
/*
521-
* WARNING: 'fpu' is dynamically-sized. It *MUST* be at
522-
* the end.
523-
*/
519+
struct fpu *fpu;
524520
};
525521

526-
#define x86_task_fpu(task) (&(task)->thread.fpu)
527-
528-
extern void fpu_thread_struct_whitelist(unsigned long *offset, unsigned long *size);
522+
#define x86_task_fpu(task) ((task)->thread.fpu)
529523

530-
static inline void arch_thread_struct_whitelist(unsigned long *offset,
531-
unsigned long *size)
524+
/*
525+
* X86 doesn't need any embedded-FPU-struct quirks:
526+
*/
527+
static inline void
528+
arch_thread_struct_whitelist(unsigned long *offset, unsigned long *size)
532529
{
533-
fpu_thread_struct_whitelist(offset, size);
530+
*offset = 0;
531+
*size = 0;
534532
}
535533

536534
static inline void

arch/x86/kernel/fpu/core.c

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -593,8 +593,19 @@ static int update_fpu_shstk(struct task_struct *dst, unsigned long ssp)
593593
int fpu_clone(struct task_struct *dst, unsigned long clone_flags, bool minimal,
594594
unsigned long ssp)
595595
{
596+
/*
597+
* We allocate the new FPU structure right after the end of the task struct.
598+
* task allocation size already took this into account.
599+
*
600+
* This is safe because task_struct size is a multiple of cacheline size.
601+
*/
596602
struct fpu *src_fpu = x86_task_fpu(current);
597-
struct fpu *dst_fpu = x86_task_fpu(dst);
603+
struct fpu *dst_fpu = (void *)dst + sizeof(*dst);
604+
605+
BUILD_BUG_ON(sizeof(*dst) % SMP_CACHE_BYTES != 0);
606+
BUG_ON(!src_fpu);
607+
608+
dst->thread.fpu = dst_fpu;
598609

599610
/* The new task's FPU state cannot be valid in the hardware. */
600611
dst_fpu->last_cpu = -1;
@@ -663,16 +674,6 @@ int fpu_clone(struct task_struct *dst, unsigned long clone_flags, bool minimal,
663674
return 0;
664675
}
665676

666-
/*
667-
* Whitelist the FPU register state embedded into task_struct for hardened
668-
* usercopy.
669-
*/
670-
void fpu_thread_struct_whitelist(unsigned long *offset, unsigned long *size)
671-
{
672-
*offset = offsetof(struct thread_struct, fpu.__fpstate.regs);
673-
*size = fpu_kernel_cfg.default_size;
674-
}
675-
676677
/*
677678
* Drops current FPU state: deactivates the fpregs and
678679
* the fpstate. NOTE: it still leaves previous contents

arch/x86/kernel/fpu/init.c

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,15 @@ static bool __init fpu__probe_without_cpuid(void)
7171
return fsw == 0 && (fcw & 0x103f) == 0x003f;
7272
}
7373

74+
static struct fpu x86_init_fpu __attribute__ ((aligned (64))) __read_mostly;
75+
7476
static void __init fpu__init_system_early_generic(void)
7577
{
78+
fpstate_reset(&x86_init_fpu);
79+
current->thread.fpu = &x86_init_fpu;
80+
set_thread_flag(TIF_NEED_FPU_LOAD);
81+
x86_init_fpu.last_cpu = -1;
82+
7683
if (!boot_cpu_has(X86_FEATURE_CPUID) &&
7784
!test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) {
7885
if (fpu__probe_without_cpuid())
@@ -150,6 +157,8 @@ static void __init fpu__init_task_struct_size(void)
150157
{
151158
int task_size = sizeof(struct task_struct);
152159

160+
task_size += sizeof(struct fpu);
161+
153162
/*
154163
* Subtract off the static size of the register state.
155164
* It potentially has a bunch of padding.
@@ -164,14 +173,9 @@ static void __init fpu__init_task_struct_size(void)
164173

165174
/*
166175
* We dynamically size 'struct fpu', so we require that
167-
* it be at the end of 'thread_struct' and that
168-
* 'thread_struct' be at the end of 'task_struct'. If
169-
* you hit a compile error here, check the structure to
170-
* see if something got added to the end.
176+
* 'state' be at the end of 'it:
171177
*/
172178
CHECK_MEMBER_AT_END_OF(struct fpu, __fpstate);
173-
CHECK_MEMBER_AT_END_OF(struct thread_struct, fpu);
174-
CHECK_MEMBER_AT_END_OF(struct task_struct, thread);
175179

176180
arch_task_struct_size = task_size;
177181
}
@@ -213,7 +217,6 @@ static void __init fpu__init_system_xstate_size_legacy(void)
213217
*/
214218
void __init fpu__init_system(void)
215219
{
216-
fpstate_reset(x86_task_fpu(current));
217220
fpu__init_system_early_generic();
218221

219222
/*

arch/x86/kernel/process.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
103103
dst->thread.vm86 = NULL;
104104
#endif
105105
/* Drop the copied pointer to current's fpstate */
106-
x86_task_fpu(dst)->fpstate = NULL;
106+
dst->thread.fpu = NULL;
107107

108108
return 0;
109109
}

include/linux/sched.h

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1646,22 +1646,15 @@ struct task_struct {
16461646
struct user_event_mm *user_event_mm;
16471647
#endif
16481648

1649-
/*
1650-
* New fields for task_struct should be added above here, so that
1651-
* they are included in the randomized portion of task_struct.
1652-
*/
1653-
randomized_struct_fields_end
1654-
16551649
/* CPU-specific state of this task: */
16561650
struct thread_struct thread;
16571651

16581652
/*
1659-
* WARNING: on x86, 'thread_struct' contains a variable-sized
1660-
* structure. It *MUST* be at the end of 'task_struct'.
1661-
*
1662-
* Do not put anything below here!
1653+
* New fields for task_struct should be added above here, so that
1654+
* they are included in the randomized portion of task_struct.
16631655
*/
1664-
};
1656+
randomized_struct_fields_end
1657+
} __attribute__ ((aligned (64)));
16651658

16661659
#define TASK_REPORT_IDLE (TASK_REPORT + 1)
16671660
#define TASK_REPORT_MAX (TASK_REPORT_IDLE << 1)

0 commit comments

Comments
 (0)