From owner-freebsd-current@FreeBSD.ORG Mon May 22 01:31:37 2006 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8549416A4EE for ; Mon, 22 May 2006 01:31:37 +0000 (UTC) (envelope-from rosti.bsd@gmail.com) Received: from ug-out-1314.google.com (ug-out-1314.google.com [66.249.92.169]) by mx1.FreeBSD.org (Postfix) with ESMTP id C773E43D6E for ; Mon, 22 May 2006 01:31:24 +0000 (GMT) (envelope-from rosti.bsd@gmail.com) Received: by ug-out-1314.google.com with SMTP id m3so1198451uge for ; Sun, 21 May 2006 18:31:22 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:date:from:to:cc:subject:message-id:in-reply-to:references:x-mailer:mime-version:content-type; b=D4dv3oA6JU81glxNumQlcdAmlhO96PsNPsoO3Ktk4QyaXVnp5vK2EawWzhQOtwcFmc7jrKye6DThUevlx+ODJXs1R9A5AkNGD/KkWUtG3Qiru4xPiHCGKtLHU74HaHNTmdO3fNNPzpp7KeWJ1L4pancV4EnJ1FE5MCbR5lzNxsE= Received: by 10.66.244.11 with SMTP id r11mr3309707ugh; Sun, 21 May 2006 18:31:22 -0700 (PDT) Received: from saturn.lan ( [212.143.154.227]) by mx.gmail.com with ESMTP id e1sm4848886ugf.2006.05.21.18.31.20; Sun, 21 May 2006 18:31:22 -0700 (PDT) Date: Mon, 22 May 2006 04:31:15 +0300 From: Rostislav Krasny To: David Xu Message-Id: <20060522043115.e581e490.rosti.bsd@gmail.com> In-Reply-To: <200605211606.43381.davidxu@freebsd.org> References: <20060430142408.fcd60069.rosti.bsd@gmail.com> <20060519210105.d4418b6f.rosti.bsd@gmail.com> <20060520012653.41cf7366.rosti.bsd@gmail.com> <200605211606.43381.davidxu@freebsd.org> X-Mailer: Sylpheed version 2.2.4 (GTK+ 2.8.17; i386-portbld-freebsd6.1) Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Multipart=_Mon__22_May_2006_04_31_15_+0300_KGhdYI=ifZv6W/Kh" Cc: Igor Sysoev , Colin Percival , freebsd-current@freebsd.org Subject: Re: [PATCH] FreeBSD-SA-06:14.fpu X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 May 2006 01:31:38 -0000 This is a multi-part message in MIME format. --Multipart=_Mon__22_May_2006_04_31_15_+0300_KGhdYI=ifZv6W/Kh Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Sun, 21 May 2006 16:06:43 +0800 David Xu wrote: > On Saturday 20 May 2006 06:26, Rostislav Krasny wrote: > > Ok, there is the patch. Attached to this email. I tested it on my i386 > > 6.1-STABLE with GENERIC and with custom MYKERNEL. MYKERNEL doesn't have > > "options CPU_FXSAVE_LEAK" and it also attached to this email. I changed > > FXSAVE_LEAK to CPU_FXSAVE_LEAK for consistency with other CPU_* options. > > I don't have any amd64 machine, so I didn't test this patch on that > > architecture. Could somebody with amd64 test it? > > > > By the way, following command could be used to check how kernel has > > been compiled, regarding the CPU_FXSAVE_LEAK option: > > > > objdump -x /boot/kernel/kernel | grep fpu_clean_state > > The patch looks fine to me, but can it be CPU_FXSAVE_NOLEAK ? > so only people know the problem will turn it on. I've changed the patch. But instead of CPU_FXSAVE_NOLEAK I use CPU_FXSAVE_NO_LEAK, for consistency with options like CPU_CYRIX_NO_LOCK. The new patch is safer for STABLE users with custom kernels because even previously customized kernels should continue to use fpu_clean_state() after rebuilding. A new version of the patch is attached to this email. If that patch is going to be commited, a few words about it should probably be written in the src/UPDATING for those who use Intel or VIA processors. --Multipart=_Mon__22_May_2006_04_31_15_+0300_KGhdYI=ifZv6W/Kh Content-Type: text/plain; name="fpu2.diff" Content-Disposition: attachment; filename="fpu2.diff" Content-Transfer-Encoding: 7bit diff -ru src/sys.orig/amd64/amd64/fpu.c src/sys/amd64/amd64/fpu.c --- src/sys.orig/amd64/amd64/fpu.c Sun Apr 23 00:16:39 2006 +++ src/sys/amd64/amd64/fpu.c Mon May 22 02:56:15 2006 @@ -33,6 +33,8 @@ #include __FBSDID("$FreeBSD: src/sys/amd64/amd64/fpu.c,v 1.157.2.1 2006/04/19 07:00:35 cperciva Exp $"); +#include "opt_cpu.h" + #include #include #include @@ -96,7 +98,9 @@ typedef u_char bool_t; +#ifndef CPU_FXSAVE_NO_LEAK static void fpu_clean_state(void); +#endif int hw_float = 1; SYSCTL_INT(_hw,HW_FLOATINGPT, floatingpoint, @@ -409,7 +413,9 @@ PCPU_SET(fpcurthread, curthread); pcb = PCPU_GET(curpcb); +#ifndef CPU_FXSAVE_NO_LEAK fpu_clean_state(); +#endif if ((pcb->pcb_flags & PCB_FPUINITDONE) == 0) { /* @@ -478,7 +484,9 @@ s = intr_disable(); if (td == PCPU_GET(fpcurthread)) { +#ifndef CPU_FXSAVE_NO_LEAK fpu_clean_state(); +#endif fxrstor(addr); intr_restore(s); } else { @@ -488,6 +496,7 @@ curthread->td_pcb->pcb_flags |= PCB_FPUINITDONE; } +#ifndef CPU_FXSAVE_NO_LEAK /* * On AuthenticAMD processors, the fxrstor instruction does not restore * the x87's stored last instruction pointer, last data pointer, and last @@ -518,6 +527,7 @@ */ __asm __volatile("ffree %%st(7); fld %0" : : "m" (dummy_variable)); } +#endif /* !CPU_FXSAVE_NO_LEAK */ /* * This really sucks. We want the acpi version only, but it requires diff -ru src/sys.orig/amd64/conf/NOTES src/sys/amd64/conf/NOTES --- src/sys.orig/amd64/conf/NOTES Mon May 1 11:47:20 2006 +++ src/sys/amd64/conf/NOTES Mon May 22 02:45:11 2006 @@ -57,6 +57,12 @@ # Options for CPU features. # +# CPU_FXSAVE_NO_LEAK disables security workaround of FPU registers leak by +# FXSAVE and FXRSTOR instructions of "7th generation" and "8th generation" +# processors manufactured by AMD. For more information read a +# FreeBSD-SA-06:14.fpu security advisory. +options CPU_FXSAVE_NO_LEAK + # # PERFMON causes the driver for Pentium/Pentium Pro performance counters # to be compiled. See perfmon(4) for more information. diff -ru src/sys.orig/conf/options.amd64 src/sys/conf/options.amd64 --- src/sys.orig/conf/options.amd64 Thu Jun 30 02:23:16 2005 +++ src/sys/conf/options.amd64 Mon May 22 02:39:36 2006 @@ -49,6 +49,7 @@ # EOF # ------------------------------- HAMMER opt_cpu.h +CPU_FXSAVE_NO_LEAK opt_cpu.h PPC_PROBE_CHIPSET opt_ppc.h PPC_DEBUG opt_ppc.h PSM_HOOKRESUME opt_psm.h diff -ru src/sys.orig/conf/options.i386 src/sys/conf/options.i386 --- src/sys.orig/conf/options.i386 Sat Jul 2 23:06:42 2005 +++ src/sys/conf/options.i386 Mon May 22 02:38:22 2006 @@ -52,6 +52,7 @@ CPU_ELAN_XTAL opt_cpu.h CPU_ENABLE_LONGRUN opt_cpu.h CPU_FASTER_5X86_FPU opt_cpu.h +CPU_FXSAVE_NO_LEAK opt_cpu.h CPU_GEODE opt_cpu.h CPU_I486_ON_386 opt_cpu.h CPU_IORT opt_cpu.h diff -ru src/sys.orig/i386/conf/NOTES src/sys/i386/conf/NOTES --- src/sys.orig/i386/conf/NOTES Thu May 11 15:41:40 2006 +++ src/sys/i386/conf/NOTES Mon May 22 02:45:36 2006 @@ -118,6 +118,11 @@ # # CPU_FASTER_5X86_FPU enables faster FPU exception handler. # +# CPU_FXSAVE_NO_LEAK disables security workaround of FPU registers leak by +# FXSAVE and FXRSTOR instructions of "7th generation" and "8th generation" +# processors manufactured by AMD. For more information read a +# FreeBSD-SA-06:14.fpu security advisory. +# # CPU_GEODE is for the SC1100 Geode embedded processor. This option # is necessary because the i8254 timecounter is toast. # @@ -192,6 +197,7 @@ options CPU_ELAN_XTAL=32768000 options CPU_ENABLE_LONGRUN options CPU_FASTER_5X86_FPU +options CPU_FXSAVE_NO_LEAK options CPU_GEODE options CPU_I486_ON_386 options CPU_IORT diff -ru src/sys.orig/i386/isa/npx.c src/sys/i386/isa/npx.c --- src/sys.orig/i386/isa/npx.c Mon May 1 11:48:01 2006 +++ src/sys/i386/isa/npx.c Mon May 22 02:54:26 2006 @@ -142,7 +142,7 @@ typedef u_char bool_t; -#ifdef CPU_ENABLE_SSE +#if defined(CPU_ENABLE_SSE) && !defined(CPU_FXSAVE_NO_LEAK) static void fpu_clean_state(void); #endif @@ -956,7 +956,7 @@ fnsave(addr); } -#ifdef CPU_ENABLE_SSE +#if defined(CPU_ENABLE_SSE) && !defined(CPU_FXSAVE_NO_LEAK) /* * On AuthenticAMD processors, the fxrstor instruction does not restore * the x87's stored last instruction pointer, last data pointer, and last @@ -987,7 +987,7 @@ */ __asm __volatile("ffree %%st(7); fld %0" : : "m" (dummy_variable)); } -#endif /* CPU_ENABLE_SSE */ +#endif /* CPU_ENABLE_SSE && !CPU_FXSAVE_NO_LEAK */ static void fpurstor(addr) @@ -996,7 +996,9 @@ #ifdef CPU_ENABLE_SSE if (cpu_fxsr) { +#ifndef CPU_FXSAVE_NO_LEAK fpu_clean_state(); +#endif fxrstor(addr); } else #endif --Multipart=_Mon__22_May_2006_04_31_15_+0300_KGhdYI=ifZv6W/Kh--