Messages in this thread Patch in this message | | | Date | Thu, 30 Mar 2017 10:01:01 +0200 | From | Borislav Petkov <> | Subject | Re: [tip:x86/mm] x86/boot/32: Rewrite test_wp_bit() |
| |
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 <luto@kernel.org> > AuthorDate: Wed, 29 Mar 2017 16:47:35 -0700 > Committer: Ingo Molnar <mingo@kernel.org> > 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 <luto@kernel.org> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Juergen Gross <jgross@suse.com> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Thomas Garnier <thgarnie@google.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Link: http://lkml.kernel.org/r/0b9e64ab0236de30e7572213cea77bf95ae2e990.1490831211.git.luto@kernel.org > Signed-off-by: Ingo Molnar <mingo@kernel.org> > --- > 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 <bp@suse.de> 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 <bp@suse.de> --- 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.
| |