linux-kernel - Re: [PATCH] x86: fix app crashes after SMP resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5d6222a80805130703n68018048se32e7feadea26dc4@mail.gmail.com>
Date:	Tue, 13 May 2008 11:03:21 -0300
From:	"Glauber Costa" <glommer@...il.com>
To:	"Hugh Dickins" <hugh@...itas.com>
Cc:	"Glauber Costa" <gcosta@...hat.com>,
	"Linus Torvalds" <torvalds@...ux-foundation.org>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Ingo Molnar" <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	"Theodore Ts'o" <tytso@....edu>,
	"Carlos R. Mafra" <crmafra2@...il.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: fix app crashes after SMP resume

On Tue, May 13, 2008 at 10:26 AM, Hugh Dickins <hugh@...itas.com> wrote:
> After resume on a 2cpu laptop, kernel builds collapse with a sed hang,
>  sh or make segfault (often on 20295564), real-time signal to cc1 etc.
>
>  Several hurdles to jump, but a manually-assisted bisect led to -rc1's
>  d2bcbad5f3ad38a1c09861bca7e252dde7bb8259 x86: do not zap_low_mappings
>  in __smp_prepare_cpus.  Though the low mappings were removed at bootup,
>  they were left behind (with Global flags helping to keep them in TLB)
>  after resume or cpu online, causing the crashes seen.
>
>  Reinstate zap_low_mappings (with local __flush_tlb_all) for each cpu_up
>  on x86_32.  This used to be serialized by smp_commenced_mask: that's now
>  gone, but a low_mappings flag will do.  No need for native_smp_cpus_done
>  to repeat the zap: let mem_init zap BSP's low mappings just like on UP.

Great!

Thanks for finding and fixing this, hugh.

>  --- 2.6.26-rc2/arch/x86/kernel/smpboot.c        2008-05-12 02:01:05.000000000 +0100
>  +++ linux/arch/x86/kernel/smpboot.c     2008-05-12 14:07:06.000000000 +0100
>  @@ -86,6 +86,7 @@ void *x86_bios_cpu_apicid_early_ptr;
>
>   #ifdef CONFIG_X86_32
>   u8 apicid_2_node[MAX_APICID];
>  +static int low_mappings;
>   #endif
>
>   /* State of each CPU */
>  @@ -326,6 +327,12 @@ static void __cpuinit start_secondary(vo
>                 enable_8259A_irq(0);
>         }
>
>  +#ifdef CONFIG_X86_32
>  +       while (low_mappings)
>  +               cpu_relax();
>  +       __flush_tlb_all();
>  +#endif
>  +

If possible, we don't want to introduce any more ifdefs. Should be
better to do it openly, define low_mappings as always 0 for x86_64. We
can use an unlikely() test to make it faster too.

>         /* This must be done before setting cpu_online_map */
>         set_cpu_sibling_map(raw_smp_processor_id());
>         wmb();
>  @@ -1040,14 +1047,20 @@ int __cpuinit native_cpu_up(unsigned int
>   #ifdef CONFIG_X86_32
>         /* init low mem mapping */
>         clone_pgd_range(swapper_pg_dir, swapper_pg_dir + KERNEL_PGD_BOUNDARY,
>  -                       min_t(unsigned long, KERNEL_PGD_PTRS, KERNEL_PGD_BOUNDARY));
>  +               min_t(unsigned long, KERNEL_PGD_PTRS, KERNEL_PGD_BOUNDARY));
>         flush_tlb_all();
>  -#endif
>  +       low_mappings = 1;
>
>         err = do_boot_cpu(apicid, cpu);
>  -       if (err < 0) {
>  +
>  +       zap_low_mappings();
>  +       low_mappings = 0;
>  +#else
>  +       err = do_boot_cpu(apicid, cpu);
>  +#endif
>  +       if (err) {
>                 Dprintk("do_boot_cpu failed %d\n", err);
>  -               return err;
>  +               return -EIO;
>         }
>
>         /*
>  @@ -1259,9 +1272,6 @@ void __init native_smp_cpus_done(unsigne
>         setup_ioapic_dest();
>   #endif
>         check_nmi_watchdog();
>  -#ifdef CONFIG_X86_32
>  -       zap_low_mappings();
>  -#endif
>   }
>
>   #ifdef CONFIG_HOTPLUG_CPU
>  --- 2.6.26-rc2/arch/x86/mm/init_32.c    2008-05-03 21:54:41.000000000 +0100
>  +++ linux/arch/x86/mm/init_32.c 2008-05-12 14:07:06.000000000 +0100
>  @@ -438,8 +438,6 @@ void zap_low_mappings(void)
>   {
>         int i;
>
>  -       save_pg_dir();
>  -
>         /*
>          * Zap initial low-memory mappings.
>          *
>  @@ -663,16 +661,8 @@ void __init mem_init(void)
>                 test_wp_bit();
>
>         cpa_init();
>  -
>  -       /*
>  -        * Subtle. SMP is doing it's boot stuff late (because it has to
>  -        * fork idle threads) - but it also needs low mappings for the
>  -        * protected-mode entry to work. We zap these entries only after
>  -        * the WP-bit has been tested.
>  -        */
>  -#ifndef CONFIG_SMP
>  +       save_pg_dir();
>         zap_low_mappings();
>  -#endif
>   }
>
>   #ifdef CONFIG_MEMORY_HOTPLUG
>



-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/