Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Juergen Gross]

On 22/05/18 05:54, Boris Ostrovsky wrote:
> We are making calls to C code (e.g. xen_prepare_pvh()) which may use
> stack canary (stored in GS segment).
> 
> Signed-off-by: Boris Ostrovsky 

With the clearing of EDX (instead using CDQ) you can add my

Reviewed-by: Juergen Gross 


Juergen

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Juergen Gross]

On 22/05/18 05:54, Boris Ostrovsky wrote:
> We are making calls to C code (e.g. xen_prepare_pvh()) which may use
> stack canary (stored in GS segment).
> 
> Signed-off-by: Boris Ostrovsky 

With the clearing of EDX (instead using CDQ) you can add my

Reviewed-by: Juergen Gross 


Juergen

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 19:10,  wrote:
> On 05/22/2018 12:32 PM, Jan Beulich wrote:
> On 22.05.18 at 18:20,  wrote:
>>> We are loading virtual address for $canary so we will always have EDX
>>> set to 0x. 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()?

I don't think so, no - there we're on the high mappings already (the
ELF note specifies the virtual address of the entry point, after all).

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 19:10,  wrote:
> On 05/22/2018 12:32 PM, Jan Beulich wrote:
> On 22.05.18 at 18:20,  wrote:
>>> We are loading virtual address for $canary so we will always have EDX
>>> set to 0x. 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()?

I don't think so, no - there we're on the high mappings already (the
ELF note specifies the virtual address of the entry point, after all).

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Boris Ostrovsky]

On 05/22/2018 12:32 PM, Jan Beulich wrote:
 On 22.05.18 at 18:20,  wrote:
>> On 05/22/2018 12:10 PM, Jan Beulich wrote:
>> On 22.05.18 at 17:15,  wrote:
 On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
 On 22.05.18 at 15:45,  wrote:
>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>>  
>> 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 name: CLTD); CDQ (AT: CLTQ)
> sign-extends EAX to EDX:EAX.
 But that would still be wrong, as it would set EDX to 0x 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 0x. 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

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Boris Ostrovsky]

On 05/22/2018 12:32 PM, Jan Beulich wrote:
 On 22.05.18 at 18:20,  wrote:
>> On 05/22/2018 12:10 PM, Jan Beulich wrote:
>> On 22.05.18 at 17:15,  wrote:
 On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
 On 22.05.18 at 15:45,  wrote:
>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>>  
>> 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 name: CLTD); CDQ (AT: CLTQ)
> sign-extends EAX to EDX:EAX.
 But that would still be wrong, as it would set EDX to 0x 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 0x. 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

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 18:20,  wrote:
> On 05/22/2018 12:10 PM, Jan Beulich wrote:
> On 22.05.18 at 17:15,  wrote:
>>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
>>> On 22.05.18 at 15:45,  wrote:
> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>  
> 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 name: CLTD); CDQ (AT: CLTQ)
 sign-extends EAX to EDX:EAX.
>>> But that would still be wrong, as it would set EDX to 0x 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 0x. 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().

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 18:20,  wrote:
> On 05/22/2018 12:10 PM, Jan Beulich wrote:
> On 22.05.18 at 17:15,  wrote:
>>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
>>> On 22.05.18 at 15:45,  wrote:
> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>  
> 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 name: CLTD); CDQ (AT: CLTQ)
 sign-extends EAX to EDX:EAX.
>>> But that would still be wrong, as it would set EDX to 0x 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 0x. 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().

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Boris Ostrovsky]

On 05/22/2018 12:10 PM, Jan Beulich wrote:
 On 22.05.18 at 17:15,  wrote:
>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
>> On 22.05.18 at 15:45,  wrote:
 On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
  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 name: CLTD); CDQ (AT: CLTQ)
>>> sign-extends EAX to EDX:EAX.
>> But that would still be wrong, as it would set EDX to 0x 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 0x. Isn't that what we want?

-borsi

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Boris Ostrovsky]

On 05/22/2018 12:10 PM, Jan Beulich wrote:
 On 22.05.18 at 17:15,  wrote:
>> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
>> On 22.05.18 at 15:45,  wrote:
 On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
  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 name: CLTD); CDQ (AT: CLTQ)
>>> sign-extends EAX to EDX:EAX.
>> But that would still be wrong, as it would set EDX to 0x 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 0x. Isn't that what we want?

-borsi

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 17:15,  wrote:
> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
> On 22.05.18 at 15:45,  wrote:
>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>>>  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 name: CLTD); CDQ (AT: CLTQ)
>> sign-extends EAX to EDX:EAX.
> 
> But that would still be wrong, as it would set EDX to 0x 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.

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 17:15,  wrote:
> On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
> On 22.05.18 at 15:45,  wrote:
>>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>>>  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 name: CLTD); CDQ (AT: CLTQ)
>> sign-extends EAX to EDX:EAX.
> 
> But that would still be wrong, as it would set EDX to 0x 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.

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Brian Gerst]

On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
 On 22.05.18 at 15:45,  wrote:
>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>>  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 name: CLTD); CDQ (AT: CLTQ)
> sign-extends EAX to EDX:EAX.

But that would still be wrong, as it would set EDX to 0x 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.

--
Brian Gerst

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Brian Gerst]

On Tue, May 22, 2018 at 9:57 AM, Jan Beulich  wrote:
 On 22.05.18 at 15:45,  wrote:
>> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>>  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 name: CLTD); CDQ (AT: CLTQ)
> sign-extends EAX to EDX:EAX.

But that would still be wrong, as it would set EDX to 0x 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.

--
Brian Gerst

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 15:45,  wrote:
> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>  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 name: CLTD); CDQ (AT: CLTQ)
sign-extends EAX to EDX:EAX.

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 15:45,  wrote:
> On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky 
>  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 name: CLTD); CDQ (AT: CLTQ)
sign-extends EAX to EDX:EAX.

Jan

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Brian Gerst]

On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky
 wrote:
> We are making calls to C code (e.g. xen_prepare_pvh()) which may use
> stack canary (stored in GS segment).
>
> Signed-off-by: Boris Ostrovsky 
> ---
>  arch/x86/xen/xen-pvh.S | 26 +-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
> index e1a5fbe..0169374 100644
> --- a/arch/x86/xen/xen-pvh.S
> +++ b/arch/x86/xen/xen-pvh.S
> @@ -54,6 +54,9 @@
>   * charge of setting up it's own stack, GDT and IDT.
>   */
>
> +#define PVH_GDT_ENTRY_CANARY   4
> +#define PVH_CANARY_SEL (PVH_GDT_ENTRY_CANARY * 8)
> +
>  ENTRY(pvh_start_xen)
> cld
>
> @@ -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).

--
Brian Gerst

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Brian Gerst]

On Mon, May 21, 2018 at 11:54 PM, Boris Ostrovsky
 wrote:
> We are making calls to C code (e.g. xen_prepare_pvh()) which may use
> stack canary (stored in GS segment).
>
> Signed-off-by: Boris Ostrovsky 
> ---
>  arch/x86/xen/xen-pvh.S | 26 +-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
> index e1a5fbe..0169374 100644
> --- a/arch/x86/xen/xen-pvh.S
> +++ b/arch/x86/xen/xen-pvh.S
> @@ -54,6 +54,9 @@
>   * charge of setting up it's own stack, GDT and IDT.
>   */
>
> +#define PVH_GDT_ENTRY_CANARY   4
> +#define PVH_CANARY_SEL (PVH_GDT_ENTRY_CANARY * 8)
> +
>  ENTRY(pvh_start_xen)
> cld
>
> @@ -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).

--
Brian Gerst

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 05:54,  wrote:
> We are making calls to C code (e.g. xen_prepare_pvh()) which may use
> stack canary (stored in GS segment).
> 
> Signed-off-by: Boris Ostrovsky 

Reviewed-by: Jan Beulich 

Re: [PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Jan Beulich]

>>> On 22.05.18 at 05:54,  wrote:
> We are making calls to C code (e.g. xen_prepare_pvh()) which may use
> stack canary (stored in GS segment).
> 
> Signed-off-by: Boris Ostrovsky 

Reviewed-by: Jan Beulich 

[PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Boris Ostrovsky]

We are making calls to C code (e.g. xen_prepare_pvh()) which may use
stack canary (stored in GS segment).

Signed-off-by: Boris Ostrovsky 
---
 arch/x86/xen/xen-pvh.S | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
index e1a5fbe..0169374 100644
--- a/arch/x86/xen/xen-pvh.S
+++ b/arch/x86/xen/xen-pvh.S
@@ -54,6 +54,9 @@
  * charge of setting up it's own stack, GDT and IDT.
  */
 
+#define PVH_GDT_ENTRY_CANARY   4
+#define PVH_CANARY_SEL (PVH_GDT_ENTRY_CANARY * 8)
+
 ENTRY(pvh_start_xen)
cld
 
@@ -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
+
call xen_prepare_pvh
 
/* startup_64 expects boot_params in %rsi. */
@@ -107,6 +116,17 @@ ENTRY(pvh_start_xen)
 
 #else /* CONFIG_X86_64 */
 
+   /* Set base address in stack canary descriptor. */
+   movl $_pa(gdt_start),%eax
+   movl $_pa(canary),%ecx
+   movw %cx, (PVH_GDT_ENTRY_CANARY * 8) + 2(%eax)
+   shrl $16, %ecx
+   movb %cl, (PVH_GDT_ENTRY_CANARY * 8) + 4(%eax)
+   movb %ch, (PVH_GDT_ENTRY_CANARY * 8) + 7(%eax)
+
+   mov $PVH_CANARY_SEL,%eax
+   mov %eax,%gs
+
call mk_early_pgtbl_32
 
mov $_pa(initial_page_table), %eax
@@ -150,9 +170,13 @@ gdt_start:
.quad GDT_ENTRY(0xc09a, 0, 0xf) /* __KERNEL_CS */
 #endif
.quad GDT_ENTRY(0xc092, 0, 0xf) /* __KERNEL_DS */
+   .quad GDT_ENTRY(0x4090, 0, 0x18)/* PVH_CANARY_SEL */
 gdt_end:
 
-   .balign 4
+   .balign 16
+canary:
+   .fill 48, 1, 0
+
 early_stack:
.fill 256, 1, 0
 early_stack_end:
-- 
2.9.3

[PATCH v4 1/2] xen/PVH: Set up GS segment for stack canary

[Boris Ostrovsky]

We are making calls to C code (e.g. xen_prepare_pvh()) which may use
stack canary (stored in GS segment).

Signed-off-by: Boris Ostrovsky 
---
 arch/x86/xen/xen-pvh.S | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
index e1a5fbe..0169374 100644
--- a/arch/x86/xen/xen-pvh.S
+++ b/arch/x86/xen/xen-pvh.S
@@ -54,6 +54,9 @@
  * charge of setting up it's own stack, GDT and IDT.
  */
 
+#define PVH_GDT_ENTRY_CANARY   4
+#define PVH_CANARY_SEL (PVH_GDT_ENTRY_CANARY * 8)
+
 ENTRY(pvh_start_xen)
cld
 
@@ -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
+
call xen_prepare_pvh
 
/* startup_64 expects boot_params in %rsi. */
@@ -107,6 +116,17 @@ ENTRY(pvh_start_xen)
 
 #else /* CONFIG_X86_64 */
 
+   /* Set base address in stack canary descriptor. */
+   movl $_pa(gdt_start),%eax
+   movl $_pa(canary),%ecx
+   movw %cx, (PVH_GDT_ENTRY_CANARY * 8) + 2(%eax)
+   shrl $16, %ecx
+   movb %cl, (PVH_GDT_ENTRY_CANARY * 8) + 4(%eax)
+   movb %ch, (PVH_GDT_ENTRY_CANARY * 8) + 7(%eax)
+
+   mov $PVH_CANARY_SEL,%eax
+   mov %eax,%gs
+
call mk_early_pgtbl_32
 
mov $_pa(initial_page_table), %eax
@@ -150,9 +170,13 @@ gdt_start:
.quad GDT_ENTRY(0xc09a, 0, 0xf) /* __KERNEL_CS */
 #endif
.quad GDT_ENTRY(0xc092, 0, 0xf) /* __KERNEL_DS */
+   .quad GDT_ENTRY(0x4090, 0, 0x18)/* PVH_CANARY_SEL */
 gdt_end:
 
-   .balign 4
+   .balign 16
+canary:
+   .fill 48, 1, 0
+
 early_stack:
.fill 256, 1, 0
 early_stack_end:
-- 
2.9.3