On Sat, Dec 25, 2010 at 12:02:58PM -0800, Philip Guenther wrote:
quoted text
> Here's a revised diff to correct the handling on amd64 and i386 of the 
> MXCSR register in frames that could be altered by users, so that a user 
> can't trick the kernel into faulting by trying to load an invalid MXCSR 
> value during the return to userspace.
> 
> Originally worked out by joshe@; this corrects the timing of the call to 
> fpuinit() for the primary CPU on amd64 so that the saved mask value is 
> initialized correctly.  It also uniformly uses 
> __attribute__((aligned(16))) do get a buffer of the correct alignment.
> 
> I've been using this on amd64 for a while and have confirmed that a test 
> program which previous crashed the box now continues just fine.  I'm away 
> from my i386 test box so we could use confirmation from someone running an 
> i386 that shows FXSR in the cpu0 flags list.
> 
> ok?
> 
> 
> Philip Guenther
> 
> Index: sys/arch/amd64/amd64/cpu.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 cpu.c
> --- sys/arch/amd64/amd64/cpu.c	27 Nov 2010 13:03:04 -0000	1.40
> +++ sys/arch/amd64/amd64/cpu.c	25 Dec 2010 19:38:48 -0000
> @@ -365,6 +365,8 @@ cpu_attach(struct device *parent, struct
>  void
>  cpu_init(struct cpu_info *ci)
>  {
> +	fpuinit(ci);
> +
>  	/* configure the CPU if needed */
>  	if (ci->cpu_setup != NULL)
>  		(*ci->cpu_setup)(ci);
> @@ -509,7 +511,6 @@ cpu_hatch(void *v)
>  	cpu_init_idt();
>  	lapic_set_lvt();
>  	gdt_init_cpu(ci);
> -	fpuinit(ci);
>  
>  	lldt(0);
>  
> Index: sys/arch/amd64/amd64/fpu.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/fpu.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 fpu.c
> --- sys/arch/amd64/amd64/fpu.c	29 Sep 2010 15:11:31 -0000	1.21
> +++ sys/arch/amd64/amd64/fpu.c	25 Dec 2010 19:38:48 -0000
> @@ -95,6 +95,8 @@
>  void fpudna(struct cpu_info *);
>  static int x86fpflags_to_siginfo(u_int32_t);
>  
> +uint32_t	fpu_mxcsr_mask;
> +
>  /*
>   * Init the FPU.
>   */
> @@ -103,6 +105,16 @@ fpuinit(struct cpu_info *ci)
>  {
>  	lcr0(rcr0() & ~(CR0_EM|CR0_TS));
>  	fninit();
> +	if (CPU_IS_PRIMARY(ci)) {
> +		struct fxsave64 fx __attribute__((aligned(16)));
> +
> +		bzero(&fx, sizeof(fx));
> +		fxsave(&fx);
> +		if (fx.fx_mxcsr_mask)
> +			fpu_mxcsr_mask = fx.fx_mxcsr_mask;
> +		else
> +			fpu_mxcsr_mask = __INITIAL_MXCSR_MASK__;
> +	}
>  	lcr0(rcr0() | (CR0_TS));
>  }
>  
> Index: sys/arch/amd64/amd64/machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 machdep.c
> --- sys/arch/amd64/amd64/machdep.c	22 Nov 2010 21:07:16 -0000	1.129
> +++ sys/arch/amd64/amd64/machdep.c	25 Dec 2010 19:38:49 -0000
> @@ -658,9 +658,11 @@ sys_sigreturn(struct proc *p, void *v, r
>  		fpusave_proc(p, 0);
>  
>  	if (ksc.sc_fpstate) {
> -		if ((error = copyin(ksc.sc_fpstate,
> -		    &p->p_addr->u_pcb.pcb_savefpu.fp_fxsave, sizeof (struct fxsave64))))
> +		struct fxsave64 *fx = &p->p_addr->u_pcb.pcb_savefpu.fp_fxsave;
> +
> +		if ((error = copyin(ksc.sc_fpstate, fx, sizeof(*fx))))
>  			return (error);
> +		fx->fx_mxcsr &= fpu_mxcsr_mask;
>  		p->p_md.md_flags |= MDP_USEDFPU;
>  	}
>  
> Index: sys/arch/amd64/amd64/process_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/process_machdep.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 process_machdep.c
> --- sys/arch/amd64/amd64/process_machdep.c	29 Sep 2010 15:11:31 -0000	1.9
> +++ sys/arch/amd64/amd64/process_machdep.c	25 Dec 2010 19:38:50 -0000
> @@ -137,7 +137,7 @@ process_read_fpregs(struct proc *p, stru
>  		frame->fx_fsw = 0x0000;
>  		frame->fx_ftw = 0xff;
>  		frame->fx_mxcsr = __INITIAL_MXCSR__;
> -		frame->fx_mxcsr_mask = __INITIAL_MXCSR_MASK__;
> +		frame->fx_mxcsr_mask = fpu_mxcsr_mask;
>  		p->p_md.md_flags |= MDP_USEDFPU;
>  	}
>  
> @@ -198,6 +198,7 @@ process_write_fpregs(struct proc *p, str
>  	}
>  
>  	memcpy(frame, &regs->fxstate, sizeof(*regs));
> +	frame->fx_mxcsr &= fpu_mxcsr_mask;
>  	return (0);
>  }
>  
> Index: sys/arch/amd64/include/fpu.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/include/fpu.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 fpu.h
> --- sys/arch/amd64/include/fpu.h	20 Nov 2010 20:11:17 -0000	1.7
> +++ sys/arch/amd64/include/fpu.h	25 Dec 2010 19:38:50 -0000
> @@ -49,6 +49,8 @@ struct savefpu {
>  struct trapframe;
>  struct cpu_info;
>  
> +extern uint32_t	fpu_mxcsr_mask;
> +
>  void fpuinit(struct cpu_info *);
>  void fpudrop(void);
>  void fpudiscard(struct proc *);
> Index: sys/arch/i386/i386/machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
> retrieving revision 1.485
> diff -u -p -r1.485 machdep.c
> --- sys/arch/i386/i386/machdep.c	2 Oct 2010 23:31:34 -0000	1.485
> +++ sys/arch/i386/i386/machdep.c	25 Dec 2010 19:38:52 -0000
> @@ -2362,9 +2362,11 @@ sys_sigreturn(struct proc *p, void *v, r
>  		npxsave_proc(p, 0);
>  
>  	if (context.sc_fpstate) {
> -		if ((error = copyin(context.sc_fpstate,
> -		    &p->p_addr->u_pcb.pcb_savefpu, sizeof (union savefpu))))
> +		union savefpu *sfp = &p->p_addr->u_pcb.pcb_savefpu;
> +		if ((error = copyin(context.sc_fpstate, sfp, sizeof(*sfp))))
>  			return (error);
> +		if (i386_use_fxsave)
> +			sfp->sv_xmm.sv_env.en_mxcsr &= fpu_mxcsr_mask;
>  		p->p_md.md_flags |= MDP_USEDFPU;
>  	}
>  
> Index: sys/arch/i386/i386/process_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/process_machdep.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 process_machdep.c
> --- sys/arch/i386/i386/process_machdep.c	29 Sep 2010 15:11:31 -0000	1.25
> +++ sys/arch/i386/i386/process_machdep.c	25 Dec 2010 19:38:52 -0000
> @@ -137,6 +137,7 @@ process_fninit_xmm(struct savexmm *sxmm)
>  	memset(sxmm, 0, sizeof(*sxmm));
>  	sxmm->sv_env.en_cw = __OpenBSD_NPXCW__;
>  	sxmm->sv_env.en_mxcsr = __INITIAL_MXCSR__;
> +	sxmm->sv_env.en_mxcsr_mask = fpu_mxcsr_mask;
>  	sxmm->sv_env.en_sw = 0x0000;
>  	sxmm->sv_env.en_tw = 0x00;
>  }
> @@ -308,6 +309,7 @@ process_write_fpregs(struct proc *p, str
>  		/* XXX Yuck. */
>  		memcpy(&s87, regs, sizeof(*regs));
>  		process_s87_to_xmm(&s87, &frame->sv_xmm);
> +		frame->sv_xmm.sv_env.en_mxcsr &= fpu_mxcsr_mask;
>  	} else
>  		memcpy(&frame->sv_87, regs, sizeof(*regs));
>  
> Index: sys/arch/i386/include/npx.h
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/include/npx.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 npx.h
> --- sys/arch/i386/include/npx.h	29 Sep 2010 15:11:31 -0000	1.15
> +++ sys/arch/i386/include/npx.h	25 Dec 2010 19:38:52 -0000
> @@ -96,7 +96,7 @@ struct envxmm {
>  	uint16_t en_fos;	/* FPU Data pointer selector */
>  	uint16_t en_rsvd2;
>  	uint32_t en_mxcsr;	/* MXCSR Register State */
> -	uint32_t en_rsvd3;
> +	uint32_t en_mxcsr_mask; /* Mask for valid MXCSR bits (may be 0) */
>  };
>  
>  /* FPU regsters in the extended save format. */
> @@ -142,6 +142,7 @@ struct	emcsts {
>   * Set Reference, pg. 3-369.
>   */
>  #define __INITIAL_MXCSR__       0x1f80
> +#define __INITIAL_MXCSR_MASK__	0xffbf
>  
>  /*
>   * The standard control word from finit is 0x37F, giving:
> @@ -157,6 +158,8 @@ void    process_s87_to_xmm(const struct 
>  
>  struct cpu_info;
>  struct trapframe;
> +
> +extern unsigned int fpu_mxcsr_mask;
>  
>  void	npxinit(struct cpu_info *);
>  void	npxtrap(struct trapframe *);
> Index: sys/arch/i386/isa/npx.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/isa/npx.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 npx.c
> --- sys/arch/i386/isa/npx.c	29 Sep 2010 15:11:31 -0000	1.53
> +++ sys/arch/i386/isa/npx.c	25 Dec 2010 19:38:52 -0000
> @@ -54,6 +54,7 @@
>  
>  #include <machine/cpu.h>
>  #include <machine/intr.h>
> +#include <machine/lock.h>
>  #include <machine/npx.h>
>  #include <machine/pio.h>
>  #include <machine/cpufunc.h>
> @@ -97,6 +98,11 @@
>  #define	clts()			__asm("clts")
>  #define	stts()			lcr0(rcr0() | CR0_TS)
>  
> +/*
> + * The mxcsr_mask for this host, taken from fxsave() on the primary CPU
> + */
> +unsigned int	fpu_mxcsr_mask;
> +
>  int npxintr(void *);
>  static int npxprobe1(struct isa_attach_args *);
>  static int x86fpflags_to_siginfo(u_int32_t);
> @@ -350,6 +356,16 @@ npxinit(struct cpu_info *ci)
>  		i386_fpu_fdivbug = 1;
>  		printf("%s: WARNING: Pentium FDIV bug detected!\n",
>  		    ci->ci_dev.dv_xname);
> +	}
> +	if (CPU_IS_PRIMARY(ci) && i386_use_fxsave) {
> +		struct savexmm xm __attribute__((aligned(16)));
> +
> +		bzero(&xm, sizeof(xm));
> +		fxsave(&xm);
> +		if (xm.sv_env.en_mxcsr_mask)
> +			fpu_mxcsr_mask = xm.sv_env.en_mxcsr_mask;
> +		else
> +			fpu_mxcsr_mask = __INITIAL_MXCSR_MASK__;
>  	}
>  	lcr0(rcr0() | (CR0_TS));
>  }
> 

Appears to work (i.e. boot) on both my amd64 and i386 boxes. The
latter has FXSR in the cpu0 flags. Just in case I am hallucinating,
the i386 dmesg is below.

.... Ken

OpenBSD 4.8-current (GENERIC.MP) #0: Sun Dec 26 08:56:56 EST 2010
    root@acer.westerback.ca:/usr/src/sys/arch/i386/compile/GENERIC.MP
cpu0: Pentium(R) Dual-Core CPU E5400 @ 2.70GHz ("GenuineIntel" 686-class) 2.71 GHz
cpu0: FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,XSAVE
real mem  = 2683523072 (2559MB)
avail mem = 2629521408 (2507MB)
mainbus0 at root
bios0 at mainbus0: AT/286+ BIOS, date 10/14/09, SMBIOS rev. 2.6 @ 0x9f400 (38 entries)
bios0: vendor American Megatrends Inc. version "P01-A4" date 10/14/2009
bios0: Acer Aspire X1800
acpi0 at bios0: rev 0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP APIC MCFG SLIC WDRT OEMB HPET NVHD AWMI SSDT
acpi0: wakeup devices PS2K(S4) PS2M(S4) NSMB(S4) USB0(S3) USB2(S3) NMAC(S5) P0P1(S4) HDAC(S4) BR10(S4) BR11(S4) BR12(S4) PWRB(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: apic clock running at 200MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Pentium(R) Dual-Core CPU E5400 @ 2.70GHz ("GenuineIntel" 686-class) 2.71 GHz
cpu1: FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,XSAVE
ioapic0 at mainbus0: apid 2 pa 0xfec00000, version 11, 24 pins
acpihpet0 at acpi0: 25000000 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 1 (P0P1)
acpiprt2 at acpi0: bus 2 (BR10)
acpiprt3 at acpi0: bus 3 (BR11)
acpiprt4 at acpi0: bus 4 (BR12)
acpicpu0 at acpi0: PSS
acpicpu1 at acpi0: PSS
acpitz0 at acpi0: critical temperature 125 degC
acpibtn0 at acpi0: PWRB
bios0: ROM list: 0xc0000/0xde00
cpu0: Enhanced SpeedStep 2701 MHz: speeds: 2700, 2003, 1603, 1203 MHz
pci0 at mainbus0 bus 0: configuration mode 1 (bios)
pchb0 at pci0 dev 0 function 0 "NVIDIA MCP73 Host" rev 0xa2
"NVIDIA MCP73 Memory" rev 0xa2 at pci0 dev 0 function 1 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 0 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 1 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 2 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 3 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 4 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 5 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 1 function 6 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 2 function 0 not configured
pcib0 at pci0 dev 3 function 0 "NVIDIA MCP73 ISA" rev 0xa2
nviic0 at pci0 dev 3 function 1 "NVIDIA MCP73 SMBus" rev 0xa1
iic0 at nviic0
spdmem0 at iic0 addr 0x50: 2GB DDR2 SDRAM non-parity PC2-6400CL5
spdmem1 at iic0 addr 0x51: 2GB DDR2 SDRAM non-parity PC2-6400CL5
iic1 at nviic0
"eeprom" at iic1 addr 0x55 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 3 function 2 not configured
"NVIDIA MCP73 Memory" rev 0xa1 at pci0 dev 3 function 4 not configured
ohci0 at pci0 dev 4 function 0 "NVIDIA MCP73 USB" rev 0xa1: apic 2 int 7 (irq 7), version 1.0, legacy support
ehci0 at pci0 dev 4 function 1 "NVIDIA MCP73 USB" rev 0xa1: apic 2 int 10 (irq 10)
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 "NVIDIA EHCI root hub" rev 2.00/1.00 addr 1
pciide0 at pci0 dev 8 function 0 "NVIDIA MCP73 IDE" rev 0xa1: DMA, channel 0 configured to compatibility, channel 1 configured to compatibility
pciide0: channel 0 disabled (no drives)
pciide0: channel 1 ignored (disabled)
azalia0 at pci0 dev 9 function 0 "NVIDIA MCP73 HD Audio" rev 0xa1: apic 2 int 11 (irq 11)
azalia0: codecs: Realtek ALC888, NVIDIA/0x8001, using Realtek ALC888
audio0 at azalia0
ppb0 at pci0 dev 10 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci1 at ppb0 bus 1
ppb1 at pci0 dev 11 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci2 at ppb1 bus 2
ppb2 at pci0 dev 12 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci3 at ppb2 bus 3
ppb3 at pci0 dev 13 function 0 "NVIDIA MCP73 PCIE" rev 0xa1
pci4 at ppb3 bus 4
vendor "VIA", unknown product 0x3403 (class serial bus subclass Firewire, rev 0x00) at pci4 dev 0 function 0 not configured
ahci0 at pci0 dev 14 function 0 "NVIDIA MCP73 AHCI" rev 0xa2: apic 2 int 5 (irq 5), AHCI 1.1
scsibus0 at ahci0: 32 targets
sd0 at scsibus0 targ 0 lun 0: <ATA, Hitachi HDT72103, ST2O> SCSI3 0/direct fixed
sd0: 305245MB, 512 bytes/sec, 625142448 sec total
cd0 at scsibus0 targ 1 lun 0: <ATAPI, DVD A DH16AASH, SA15> ATAPI 5/cdrom removable
nfe0 at pci0 dev 15 function 0 "NVIDIA MCP73 LAN" rev 0xa2: apic 2 int 7 (irq 7), address 00:25:11:8e:a6:7c
rgephy0 at nfe0 phy 3: RTL8169S/8110S PHY, rev. 2
vga1 at pci0 dev 16 function 0 "NVIDIA GeForce 7100" rev 0xa2
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
isa0 at pcib0
isadma0 at isa0
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
pckbc0 at isa0 port 0x60/5
pckbd0 at pckbc0 (kbd slot)
pckbc0: using irq 1 for kbd slot
wskbd0 at pckbd0: console keyboard, using wsdisplay0
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
it0 at isa0 port 0x2e/2: IT8718F rev 4, EC port 0xa10
npx0 at isa0 port 0xf0/16: reported by CPUID; using exception 16
usb1 at ohci0: USB revision 1.0
uhub1 at usb1 "NVIDIA OHCI root hub" rev 1.00/1.00 addr 1
mtrr: Pentium Pro MTRR support
umass0 at uhub0 port 6 configuration 1 interface 0 "Generic Mass Storage Device" rev 2.00/1.00 addr 2
umass0: using SCSI over Bulk-Only
scsibus1 at umass0: 2 targets, initiator 0
sd1 at scsibus1 targ 1 lun 0: <Generic-, Compact Flash, 1.01> SCSI0 0/direct removable
sd1: drive offline
sd2 at scsibus1 targ 1 lun 1: <Multiple, Flash Reader, 1.05> SCSI0 0/direct removable
sd2: drive offline
uhidev0 at uhub0 port 6 configuration 1 interface 1 "Generic Mass Storage Device" rev 2.00/1.00 addr 2
uhidev0: iclass 3/0, 4 report ids
uhid0 at uhidev0 reportid 4: input=7, output=0, feature=0
vscsi0 at root
scsibus2 at vscsi0: 256 targets
softraid0 at root
root on sd0a swap on sd0b dump on sd0b