From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933335AbdC3IBP (ORCPT ); Thu, 30 Mar 2017 04:01:15 -0400 Received: from mail.skyhub.de ([5.9.137.197]:43058 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933084AbdC3IBN (ORCPT ); Thu, 30 Mar 2017 04:01:13 -0400 Date: Thu, 30 Mar 2017 10:01:01 +0200 From: Borislav Petkov To: jgross@suse.com, boris.ostrovsky@oracle.com, torvalds@linux-foundation.org, mingo@kernel.org, peterz@infradead.org, luto@kernel.org, thgarnie@google.com, tglx@linutronix.de, hpa@zytor.com, linux-kernel@vger.kernel.org Cc: linux-tip-commits@vger.kernel.org Subject: Re: [tip:x86/mm] x86/boot/32: Rewrite test_wp_bit() Message-ID: <20170330080101.ywsf5rg6ilzu4itk@pd.tnic> References: <0b9e64ab0236de30e7572213cea77bf95ae2e990.1490831211.git.luto@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 30, 2017 at 12:29:18AM -0700, tip-bot for Andy Lutomirski wrote: > Commit-ID: 4af171105144a6475704c1e6024132883d50499e > Gitweb: http://git.kernel.org/tip/4af171105144a6475704c1e6024132883d50499e > Author: Andy Lutomirski > AuthorDate: Wed, 29 Mar 2017 16:47:35 -0700 > Committer: Ingo Molnar > CommitDate: Thu, 30 Mar 2017 09:08:33 +0200 > > x86/boot/32: Rewrite test_wp_bit() > > This code seems to be very old and has gotten only minor updates. > It's overcomplicated and has a bunch of comments that are, at best, > of purely historical interest. Nowadays we have a shiny function > probe_kernel_write() that does more or less exactly what we need. > Use it. > > I switched the page that we test from swapper_pg_dir to > empty_zero_page because writing zero to empty_zero_page is more > obviously safe than writing to the paging structures. (It's > extremely unlikely that any of this would cause problems in practice > because the write will fail on any supported CPU.) > > Signed-off-by: Andy Lutomirski > Cc: Boris Ostrovsky > Cc: Borislav Petkov > Cc: Juergen Gross > Cc: Linus Torvalds > Cc: Peter Zijlstra > Cc: Thomas Garnier > Cc: Thomas Gleixner > Link: http://lkml.kernel.org/r/0b9e64ab0236de30e7572213cea77bf95ae2e990.1490831211.git.luto@kernel.org > Signed-off-by: Ingo Molnar > --- > arch/x86/mm/init_32.c | 41 +++++++---------------------------------- > 1 file changed, 7 insertions(+), 34 deletions(-) > > diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c > index 7116a72..097089a 100644 > --- a/arch/x86/mm/init_32.c > +++ b/arch/x86/mm/init_32.c > @@ -56,8 +56,6 @@ > > unsigned long highstart_pfn, highend_pfn; > > -static noinline int do_test_wp_bit(void); > - > bool __read_mostly __vmalloc_start_set = false; > > /* > @@ -726,22 +724,21 @@ void __init paging_init(void) > */ > static void __init test_wp_bit(void) > { > - int wp_works_ok; > + char z = 0; > > printk(KERN_INFO > "Checking if this processor honours the WP bit even in supervisor mode..."); > > - /* Any page-aligned address will do, the test is non-destructive */ > - __set_fixmap(FIX_WP_TEST, __pa(&swapper_pg_dir), PAGE_KERNEL_RO); > - wp_works_ok = do_test_wp_bit(); > - clear_fixmap(FIX_WP_TEST); > + __set_fixmap(FIX_WP_TEST, __pa_symbol(empty_zero_page), PAGE_KERNEL_RO); > > - if (!wp_works_ok) { > + if (probe_kernel_write((char *)fix_to_virt(FIX_WP_TEST), &z, 1) == 0) { Gah, I was just commenting on that but you committed it already... Oh well, let's flip that logic so that we have the success case - which we exec most of the time - if not always - not out of line but put the panic() call at the end instead. A natural "likely" if you will :-) --- From: Borislav Petkov Date: Thu, 30 Mar 2017 09:44:05 +0200 Subject: [PATCH] x86/boot/32: Flip the logic in test_wp_bit() ... to have a natural "likely()" in the code flow and thus have the success case with a branch 99.999% of the times non-taken and function return code following it instead of jumping to it each time. This puts the panic() call at the end of the function - it is going to be practically unreachable anyway. The C code is a bit more readable too. No functionality change. Signed-off-by: Borislav Petkov --- arch/x86/mm/init_32.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c index 097089a5e4d5..601b8e04e5c6 100644 --- a/arch/x86/mm/init_32.c +++ b/arch/x86/mm/init_32.c @@ -726,19 +726,18 @@ static void __init test_wp_bit(void) { char z = 0; - printk(KERN_INFO - "Checking if this processor honours the WP bit even in supervisor mode..."); + printk(KERN_INFO "Checking if this processor honours the WP bit even in supervisor mode..."); __set_fixmap(FIX_WP_TEST, __pa_symbol(empty_zero_page), PAGE_KERNEL_RO); - if (probe_kernel_write((char *)fix_to_virt(FIX_WP_TEST), &z, 1) == 0) { - printk(KERN_CONT "No.\n"); - panic("Linux doesn't support CPUs with broken WP."); + if (probe_kernel_write((char *)fix_to_virt(FIX_WP_TEST), &z, 1)) { + clear_fixmap(FIX_WP_TEST); + printk(KERN_CONT "Ok.\n"); + return; } - clear_fixmap(FIX_WP_TEST); - - printk(KERN_CONT "Ok.\n"); + printk(KERN_CONT "No.\n"); + panic("Linux doesn't support CPUs with broken WP."); } void __init mem_init(void) -- 2.11.0 -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.