Messages in this thread | | | Date | Mon, 31 Jan 2011 15:52:31 -0800 | From | Kees Cook <> | Subject | Re: [BUG] broken ebba638ae723d8a8fc2f7abce5ec18b688b791d7 |
| |
Hi Matthieu,
On Tue, Feb 01, 2011 at 12:11:16AM +0100, matthieu castet wrote: > Kees Cook a écrit : > >On Thu, Jan 27, 2011 at 06:24:14PM -0800, H. Peter Anvin wrote: > >>On 01/27/2011 03:00 PM, Kees Cook wrote: > >>>Yikes, good catch. > >>> > >>>arch/x86/kernel/trampoline_64.S uses: > >>> movw $(trampoline_stack_end - r_base), %sp > >>> > >>>arch/x86/boot/compressed/head_64.S uses: > >>> movl $boot_stack_end, %eax > >>> addl %ebp, %eax > >>> movl %eax, %esp > >>> > >>>what would be safe for arch/x86/kernel/head_32.S ? It uses "stack_start", > >>>but later after paging set-up. Is the following sane to solve this? > >>> > >>To run it before paging is set up, you can't use stack, start; you > >>have to use a pointer based on physical address. You have two > >>problems with using stack_start: you're using a linear address to > >>access stack_start, and stack_start itself contains a linear > >>address. > >> > >>It's not entirely clear to me why we don't initialize %ss to > >>__BOOT_DS with the other segment registers, but it would make most > >>sense to me: > >> > >>diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S > >>index fc293dc..c10f9ba 100644 > >>--- a/arch/x86/kernel/head_32.S > >>+++ b/arch/x86/kernel/head_32.S > >>@@ -99,7 +99,12 @@ ENTRY(startup_32) > >> movl %eax,%es > >> movl %eax,%fs > >> movl %eax,%gs > >>+ movl %eax,%ss > >> 2: > >>+/* > >>+ * Set up an initial stack > >>+ */ > >>+ movl $pa(init_thread_union+THREAD_SIZE), %esp > >> > >> /* > >> * Clear BSS first so that there are no surprises... > > > >This doesn't appear to work for me. While I can boot fine, doing CPU > >hotplugging hangs the system. :( > > > This is weird because the patch only touch first cpu (startup_32 > entry) and cpu hotplug go to startup_32_smp. > > Here a untested patch that move the stack setup in the common path. > > diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S > index fc293dc..5df3432 100644 > --- a/arch/x86/kernel/head_32.S > +++ b/arch/x86/kernel/head_32.S > @@ -284,6 +284,12 @@ ENTRY(startup_32_smp) > movl %eax,%gs > #endif /* CONFIG_SMP */ > default_entry: > + /* > + * Set up an initial stack > + */ > + movl $(__BOOT_DS),%eax > + movl %eax,%ss > + movl $pa(init_thread_union+THREAD_SIZE), %esp > > /* > * New page tables may be in 4Mbyte page mode and may
This worked, thanks! If this tests cleanly for you in qemu, we should get this committed.
-Kees
-- Kees Cook Ubuntu Security Team -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |