Re: [BUG] broken ebba638ae723d8a8fc2f7abce5ec18b688b791d7

[Rafael J. Wysocki]

On Tuesday, February 01, 2011, matthieu castet wrote:
> Kees Cook a écrit :
> > Hi,
> > 
> > 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.

Well, in my not so humble opinion the amount of random fixes required by the
entire NX protection of kernel "data" pages is simply unacceptable.

Please revert the whole thing and do it once again from scratch and possibly
without breaking stuff left and right.  I guess you've learnt enough so far
that this should be doable for 2.6.39 without introducing major issues like
_boot_ problems (let alone the broken resume).

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/