d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

x86: initialize use_xsave once

Browse Source 
The explanation from https://reviews.freebsd.org/D39637 by stevek:
The "use_xsave" variable is a global and that is only supposed to be
initialized early before scheduling gets started. However, with the way
the ifuncs for "fpusave" and "fpurestore" are implemented, the value
could be changed at runtime when scheduling is active if "use_xsave"
was set to 0 by the tunable. This leaves a window of opportunity where
"use_xsave" gets re-initialized to 1 and a context switch could occur
with a thread that was not set up to be able to use xsave functionality.
This can lead to an "privileged instruction fault".

The fix is to protect "use_xsave" from being initialized more than once.

Reported and reviewed by:	stevek
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D39660
This commit is contained in:
Konstantin Belousov 2023-04-18 18:50:26 +03:00
parent 93ca6ff295
commit 617a11eab6
4 changed files with 11 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
sys/amd64/amd64/fpu.c
View File

@ -238,22 +238,8 @@ fpurestore_fxrstor(void *addr)
fxrstor((char *)addr);
}
static void
init_xsave(void)
{
if (use_xsave)
return;
if ((cpu_feature2 & CPUID2_XSAVE) == 0)
return;
use_xsave = 1;
TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
}
DEFINE_IFUNC(, void, fpusave, (void *))
{
init_xsave();
if (!use_xsave)
return (fpusave_fxsave);
if ((cpu_stdext_feature & CPUID_EXTSTATE_XSAVEOPT) != 0) {
@ -266,8 +252,6 @@ DEFINE_IFUNC(, void, fpusave, (void *))
DEFINE_IFUNC(, void, fpurestore, (void *))
{
init_xsave();
if (!use_xsave)
return (fpurestore_fxrstor);
return ((cpu_stdext_feature & CPUID_STDEXT_NFPUSG) != 0 ?

5
sys/amd64/amd64/machdep.c
View File

@ -1345,6 +1345,11 @@ hammer_time(u_int64_t modulep, u_int64_t physfree)
&pmap_pcid_invlpg_workaround_uena);
cpu_init_small_core();
if ((cpu_feature2 & CPUID2_XSAVE) != 0) {
use_xsave = 1;
TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
}
link_elf_ireloc(kmdp);
/*

6
sys/i386/i386/machdep.c
View File

@ -1545,6 +1545,11 @@ init386(int first)
i386_kdb_init();
}
if (cpu_fxsr && (cpu_feature2 & CPUID2_XSAVE) != 0) {
use_xsave = 1;
TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
}
kmdp = preload_search_by_type("elf kernel");
link_elf_ireloc(kmdp);
@ -1565,6 +1570,7 @@ init386(int first)
msgbufinit(msgbufp, msgbufsize);
npxinit(true);
/*
* Set up thread0 pcb after npxinit calculated pcb + fpu save
* area size. Zero out the extended state header in fpu save

14
sys/i386/i386/npx.c
View File

@ -320,22 +320,8 @@ fpusave_fnsave(union savefpu *addr)
fnsave((char *)addr);
}
static void
init_xsave(void)
{
if (use_xsave)
return;
if (!cpu_fxsr || (cpu_feature2 & CPUID2_XSAVE) == 0)
return;
use_xsave = 1;
TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
}
DEFINE_IFUNC(, void, fpusave, (union savefpu *))
{
init_xsave();
if (use_xsave)
return ((cpu_stdext_feature & CPUID_EXTSTATE_XSAVEOPT) != 0 ?
fpusave_xsaveopt : fpusave_xsave);