Messages in this thread | | | Subject | Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary | From | Boris Ostrovsky <> | Date | Tue, 22 May 2018 13:10:09 -0400 |
| |
On 05/22/2018 12:32 PM, Jan Beulich wrote: >>>> On 22.05.18 at 18:20, <boris.ostrovsky@oracle.com> wrote: >> On 05/22/2018 12:10 PM, Jan Beulich wrote: >>>>>> On 22.05.18 at 17:15, <brgerst@gmail.com> wrote: >>>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>>>>>> On 22.05.18 at 15:45, <brgerst@gmail.com> wrote: >>>>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky <boris.ostrovsky@oracle.com> >> wrote: >>>>>>> @@ -98,6 +101,12 @@ ENTRY(pvh_start_xen) >>>>>>> /* 64-bit entry point. */ >>>>>>> .code64 >>>>>>> 1: >>>>>>> + /* Set base address in stack canary descriptor. */ >>>>>>> + mov $MSR_GS_BASE,%ecx >>>>>>> + mov $canary, %rax >>>>>>> + cdq >>>>>>> + wrmsr >>>>>> CDQ only sign-extends EAX to RAX. What you really want is to move the >>>>>> high 32-bits to EDX (or zero EDX if we can guarantee it is loaded >>>>>> below 4G). >>>>> What you describe is CDQE (AT&T name: CLTD); CDQ (AT&T: CLTQ) >>>>> sign-extends EAX to EDX:EAX. >>>> But that would still be wrong, as it would set EDX to 0xFFFFFFFF if >>>> the kernel was loaded between 2G and 4G. Looking closer at the code, >>>> we just left 32-bit mode, so we must have been loaded below 4G, >>>> therefore EDX must be zero. >>> Ah, yes, indeed. >> We are loading virtual address for $canary so we will always have EDX >> set to 0xffffffff. Isn't that what we want? > Oh, that's rather confusing - we're still running on the low 1:1 > mapping when we're here. But yes, by the time we enter C code > (where the GS base starts to matter) we ought to be on the high > mappings - if only there wasn't xen_prepare_pvh().
xen_prepare_pvh() (and whatever it might call) is the only reason for this patch to exist. It's the only C call that we are making before jumping to startup_64, which I assume will have to set up GS itself before calling into C.
I didn't realize we are still on identity mapping. I'll clear EDX (and load $_pa(canary)) then.
BTW, don't we have the same issue in startup_xen()?
-boris
| |