Skip to contentSkip to conversation

x86: Reset FPU on exec

PATCH In review Andi Kleen 3 participants 4 messages Apr 18, 2015 – Apr 21, 2015

Subsystems (2)
x86 exec and binfmt
lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 x86: Reset FPU on exec

From: Andi Kleen <[email protected]>

Currently we don't reset FPU state on exec. This can be seen as a (minor) security issue. The bigger issue however is that the AVX state also does not get reset. So a program that uses SSE without VZEROUPPER may get a large penalty.

Always set the FPU to the init state at exec time.

For the eager FPU case this restores the init state, for non eager it forces an init on the next FPU use.

Signed-off-by Andi Kleen <[email protected]>
---
 arch/x86/include/asm/elf.h | 4 ++++
 arch/x86/kernel/xsave.c    | 5 +++++
 2 files changed, 9 insertions(+)
arch/x86/include/asm/elf.h +4 -0
index ca3347a..56ab629 100644
@@ -90,6 +90,8 @@ extern unsigned int vdso32_enabled;
 
 #include <asm/processor.h>
 
+extern void reset_fpu(void);
+
 #ifdef CONFIG_X86_32
 #include <asm/desc.h>
 

@@ -110,6 +112,7 @@ extern unsigned int vdso32_enabled;
 	_r->bx = 0; _r->cx = 0; _r->dx = 0;	\
 	_r->si = 0; _r->di = 0; _r->bp = 0;	\
 	_r->ax = 0;				\
+	reset_fpu();				\
 } while (0)
 
 /*

@@ -178,6 +181,7 @@ static inline void elf_common_init(struct thread_struct *t,
 	t->fs = t->gs = 0;
 	t->fsindex = t->gsindex = 0;
 	t->ds = t->es = ds;
+	reset_fpu();
 }
 
 #define ELF_PLAT_INIT(_r, load_addr)			\
arch/x86/kernel/xsave.c +5 -0
index cdc6cf9..520e505 100644
@@ -741,3 +741,8 @@ void *get_xsave_addr(struct xsave_struct *xsave, int xstate)
 	return (void *)xsave + xstate_comp_offsets[feature];
 }
 EXPORT_SYMBOL_GPL(get_xsave_addr);
+
+void reset_fpu(void)
+{
+	drop_init_fpu(current);
+}
Signature · 1 line
1.9.3
#1 lore raw

arch/x86/kernel/xsave.c void *get_xsave_addr(struct xsave_struct *xsave, int xstate) · line 748

Quoted 4 lines · Andi Kleen
arch/x86/kernel/xsave.c +4 -0
+void reset_fpu(void)
+{
+	drop_init_fpu(current);
+}

So we have a new function which is merily a wrapper around a non existent function.

What's the exact purpose of this patch aside of breaking the build?

May I recommend reading and following Documentation/SubmittingPatches?

Thanks,

tglx

#2 lore raw
Quoted 9 lines · Thomas Gleixner

arch/x86/kernel/xsave.c void *get_xsave_addr(struct xsave_struct *xsave, int xstate) · line 748

Quoted 4 lines · Andi Kleen
arch/x86/kernel/xsave.c +4 -0
+void reset_fpu(void)
+{
+	drop_init_fpu(current);
+}

So we have a new function which is merily a wrapper around a non existent function.

I originally tried to use the inline function directly, but it's defined in "fpu-internal.h" and including it in asm/elf.h causing all kinds of issues.

Quoted 2 lines

What's the exact purpose of this patch aside of breaking the build?

Try reading the description?

Also it builds fine here.

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only
#3 lore raw

Andi,

Quoted 3 lines · Andi Kleen
Quoted 1 line

What's the exact purpose of this patch aside of breaking the build?

Try reading the description?

I'll try to decrypt that once I have a functional patch.

Quoted 1 line

Also it builds fine here.

I don't care whether it builds for you on your random tree. I care whether it compiles against the relevant reference trees (tip/linus).

And it CANNOT compile against them at all. Simply because drop_init_fpu() does not exist.

You are just out there to waste maintainer time and annoy everyone who has the displeasure to handle your sloppy patches and deal with your obnoxious unwillingness to cooperate, aren't you?

Thanks,

tglx