linux-kernel - Re: [Xen-devel] [PATCH 3/4] xen/PVH: Set up GS segment for stack canary

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <5AE9D2DD02000078001C02F0@prv1-mh.provo.novell.com>
Date:   Wed, 02 May 2018 09:01:49 -0600
From:   "Jan Beulich" <JBeulich@...e.com>
To:     "Boris Ostrovsky" <boris.ostrovsky@...cle.com>
Cc:     "xen-devel" <xen-devel@...ts.xenproject.org>,
        "Juergen Gross" <jgross@...e.com>, <linux-kernel@...r.kernel.org>,
        <stable@...r.kernel.org>
Subject: Re: [Xen-devel] [PATCH 3/4] xen/PVH: Set up GS segment for
 stack canary

>>> On 02.05.18 at 17:00, <boris.ostrovsky@...cle.com> wrote:
> On 05/02/2018 04:16 AM, Jan Beulich wrote:
>>>>> On 30.04.18 at 18:23, <boris.ostrovsky@...cle.com> wrote:
>>> --- 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)
>> I can only advise against doing it this way: There's no safeguard against
>> someone changing asm/segment.h without changing this value (in fact
>> this applies to all of the GDT selectors populated in this file). At the 
> very
>> least tie this to GDT_ENTRY_BOOT_TSS / __BOOT_TSS?
>>
>>> @@ -64,6 +67,9 @@ ENTRY(pvh_start_xen)
>>>  	mov %eax,%es
>>>  	mov %eax,%ss
>>>  
>>> +	mov $(PVH_CANARY_SEL),%eax
>>> +	mov %eax,%gs
>>> +
>>>  	/* Stash hvm_start_info. */
>>>  	mov $_pa(pvh_start_info), %edi
>>>  	mov %ebx, %esi
>>> @@ -150,6 +156,7 @@ gdt_start:
>>>  	.quad 0x00cf9a000000ffff            /* __BOOT_CS */
>>>  #endif
>>>  	.quad 0x00cf92000000ffff            /* __BOOT_DS */
>>> +	.quad 0x0040900000000018            /* PVH_CANARY_SEL */
>> Without any further code before loading the selector, this points at
>> physical address 0. Don't you need to add in the base address of
>> the per-CPU stack_canary?
> 
> This GDT is gone soon after we jump into generic x86 startup code.That
> code will load its own GDT (and then set up per-cpu segments and all that).

All understood, but why would you set up the per-CPU segment here if
what you load into the segment register is not usable for the intended
purpose (until that other code sets up things and reloads the segment
registers)?

Jan