* [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
@ 2018-04-09 9:44 Andrew Cooper
2018-04-09 10:44 ` Wei Liu
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Cooper @ 2018-04-09 9:44 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
wrong constant to use. Switch to FLAT_USER_SS32.
For compat domains however, the reported values are entirely bogus.
FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
The guests SYSCALL callback is invoked with a broken iret frame, and if left
unmodified by the guest, will fail on the way back out when Xen's iret tries
to load a code segment into %ss.
In practice, this is only a problem for 32bit PV guests on AMD hardware, as
Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
This appears to have been broken ever since 64bit support was added to Xen,
and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Juergen Gross <jgross@suse.com>
This wants backporting basically everywhere, and as such, also wants to be
considered for 4.11 at this point.
---
xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index 6c7fcf9..2bc046c 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -197,7 +197,7 @@ ENTRY(cstar_enter)
/* sti could live here when we don't switch page tables below. */
CR4_PV32_RESTORE
movq 8(%rsp),%rax /* Restore %rax. */
- movq $FLAT_KERNEL_SS,8(%rsp)
+ movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
pushq %r11
pushq $FLAT_USER_CS32
pushq %rcx
@@ -223,6 +223,11 @@ ENTRY(cstar_enter)
movq VCPU_domain(%rbx),%rcx
cmpb $0,DOMAIN_is_32bit_pv(%rcx)
je switch_to_kernel
+
+ /* Fix up reported %cs/%ss for compat domains. */
+ movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
+ movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
+
cmpb $0,VCPU_syscall32_disables_events(%rbx)
movzwl VCPU_syscall32_sel(%rbx),%esi
movq VCPU_syscall32_addr(%rbx),%rax
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
2018-04-09 9:44 [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry Andrew Cooper
@ 2018-04-09 10:44 ` Wei Liu
2018-04-09 10:46 ` Andrew Cooper
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Wei Liu @ 2018-04-09 10:44 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Mon, Apr 09, 2018 at 10:44:47AM +0100, Andrew Cooper wrote:
> The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
> wrong constant to use. Switch to FLAT_USER_SS32.
>
> For compat domains however, the reported values are entirely bogus.
> FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
> FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
>
> The guests SYSCALL callback is invoked with a broken iret frame, and if left
> unmodified by the guest, will fail on the way back out when Xen's iret tries
> to load a code segment into %ss.
>
> In practice, this is only a problem for 32bit PV guests on AMD hardware, as
> Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
>
> This appears to have been broken ever since 64bit support was added to Xen,
> and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
>
> This wants backporting basically everywhere, and as such, also wants to be
> considered for 4.11 at this point.
> ---
> xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
> index 6c7fcf9..2bc046c 100644
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -197,7 +197,7 @@ ENTRY(cstar_enter)
> /* sti could live here when we don't switch page tables below. */
> CR4_PV32_RESTORE
> movq 8(%rsp),%rax /* Restore %rax. */
> - movq $FLAT_KERNEL_SS,8(%rsp)
> + movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
> pushq %r11
> pushq $FLAT_USER_CS32
> pushq %rcx
> @@ -223,6 +223,11 @@ ENTRY(cstar_enter)
> movq VCPU_domain(%rbx),%rcx
> cmpb $0,DOMAIN_is_32bit_pv(%rcx)
> je switch_to_kernel
> +
> + /* Fix up reported %cs/%ss for compat domains. */
> + movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
> + movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
I wonder if it would be better to introduce COMPAT_FLAT_RING3_* in
xen-x86_64.h?
In any case, the reasoning and code looks correct to me:
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
2018-04-09 10:44 ` Wei Liu
@ 2018-04-09 10:46 ` Andrew Cooper
2018-04-09 10:49 ` Juergen Gross
2018-04-09 10:47 ` Juergen Gross
2018-04-09 12:02 ` Jan Beulich
2 siblings, 1 reply; 8+ messages in thread
From: Andrew Cooper @ 2018-04-09 10:46 UTC (permalink / raw)
To: Wei Liu; +Cc: Juergen Gross, Roger Pau Monné, Jan Beulich, Xen-devel
On 09/04/18 11:44, Wei Liu wrote:
> On Mon, Apr 09, 2018 at 10:44:47AM +0100, Andrew Cooper wrote:
>> The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
>> wrong constant to use. Switch to FLAT_USER_SS32.
>>
>> For compat domains however, the reported values are entirely bogus.
>> FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
>> FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
>>
>> The guests SYSCALL callback is invoked with a broken iret frame, and if left
>> unmodified by the guest, will fail on the way back out when Xen's iret tries
>> to load a code segment into %ss.
>>
>> In practice, this is only a problem for 32bit PV guests on AMD hardware, as
>> Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
>>
>> This appears to have been broken ever since 64bit support was added to Xen,
>> and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>>
>> This wants backporting basically everywhere, and as such, also wants to be
>> considered for 4.11 at this point.
>> ---
>> xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
>> index 6c7fcf9..2bc046c 100644
>> --- a/xen/arch/x86/x86_64/compat/entry.S
>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>> @@ -197,7 +197,7 @@ ENTRY(cstar_enter)
>> /* sti could live here when we don't switch page tables below. */
>> CR4_PV32_RESTORE
>> movq 8(%rsp),%rax /* Restore %rax. */
>> - movq $FLAT_KERNEL_SS,8(%rsp)
>> + movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
>> pushq %r11
>> pushq $FLAT_USER_CS32
>> pushq %rcx
>> @@ -223,6 +223,11 @@ ENTRY(cstar_enter)
>> movq VCPU_domain(%rbx),%rcx
>> cmpb $0,DOMAIN_is_32bit_pv(%rcx)
>> je switch_to_kernel
>> +
>> + /* Fix up reported %cs/%ss for compat domains. */
>> + movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
>> + movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
> I wonder if it would be better to introduce COMPAT_FLAT_RING3_* in
> xen-x86_64.h?
>
> In any case, the reasoning and code looks correct to me:
>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
I considered that, and am open to suggestions. The problem is that we
don't want to put COMPAT definitions in that header file, because they
are inapplicable to 64bit PV guests, who are intended consumers.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
2018-04-09 10:44 ` Wei Liu
2018-04-09 10:46 ` Andrew Cooper
@ 2018-04-09 10:47 ` Juergen Gross
2018-04-09 12:02 ` Jan Beulich
2 siblings, 0 replies; 8+ messages in thread
From: Juergen Gross @ 2018-04-09 10:47 UTC (permalink / raw)
To: Wei Liu, Andrew Cooper; +Cc: Xen-devel, Jan Beulich, Roger Pau Monné
On 09/04/18 12:44, Wei Liu wrote:
> On Mon, Apr 09, 2018 at 10:44:47AM +0100, Andrew Cooper wrote:
>> The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
>> wrong constant to use. Switch to FLAT_USER_SS32.
>>
>> For compat domains however, the reported values are entirely bogus.
>> FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
>> FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
>>
>> The guests SYSCALL callback is invoked with a broken iret frame, and if left
>> unmodified by the guest, will fail on the way back out when Xen's iret tries
>> to load a code segment into %ss.
>>
>> In practice, this is only a problem for 32bit PV guests on AMD hardware, as
>> Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
>>
>> This appears to have been broken ever since 64bit support was added to Xen,
>> and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>>
>> This wants backporting basically everywhere, and as such, also wants to be
>> considered for 4.11 at this point.
>> ---
>> xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
>> index 6c7fcf9..2bc046c 100644
>> --- a/xen/arch/x86/x86_64/compat/entry.S
>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>> @@ -197,7 +197,7 @@ ENTRY(cstar_enter)
>> /* sti could live here when we don't switch page tables below. */
>> CR4_PV32_RESTORE
>> movq 8(%rsp),%rax /* Restore %rax. */
>> - movq $FLAT_KERNEL_SS,8(%rsp)
>> + movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
>> pushq %r11
>> pushq $FLAT_USER_CS32
>> pushq %rcx
>> @@ -223,6 +223,11 @@ ENTRY(cstar_enter)
>> movq VCPU_domain(%rbx),%rcx
>> cmpb $0,DOMAIN_is_32bit_pv(%rcx)
>> je switch_to_kernel
>> +
>> + /* Fix up reported %cs/%ss for compat domains. */
>> + movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
>> + movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
>
> I wonder if it would be better to introduce COMPAT_FLAT_RING3_* in
> xen-x86_64.h?
That was my first thought, too.
>
> In any case, the reasoning and code looks correct to me:
>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
2018-04-09 10:46 ` Andrew Cooper
@ 2018-04-09 10:49 ` Juergen Gross
2018-04-09 12:01 ` Andrew Cooper
0 siblings, 1 reply; 8+ messages in thread
From: Juergen Gross @ 2018-04-09 10:49 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu; +Cc: Roger Pau Monné, Jan Beulich, Xen-devel
On 09/04/18 12:46, Andrew Cooper wrote:
> On 09/04/18 11:44, Wei Liu wrote:
>> On Mon, Apr 09, 2018 at 10:44:47AM +0100, Andrew Cooper wrote:
>>> The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
>>> wrong constant to use. Switch to FLAT_USER_SS32.
>>>
>>> For compat domains however, the reported values are entirely bogus.
>>> FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
>>> FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
>>>
>>> The guests SYSCALL callback is invoked with a broken iret frame, and if left
>>> unmodified by the guest, will fail on the way back out when Xen's iret tries
>>> to load a code segment into %ss.
>>>
>>> In practice, this is only a problem for 32bit PV guests on AMD hardware, as
>>> Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
>>>
>>> This appears to have been broken ever since 64bit support was added to Xen,
>>> and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>> CC: Wei Liu <wei.liu2@citrix.com>
>>> CC: Juergen Gross <jgross@suse.com>
>>>
>>> This wants backporting basically everywhere, and as such, also wants to be
>>> considered for 4.11 at this point.
>>> ---
>>> xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
>>> index 6c7fcf9..2bc046c 100644
>>> --- a/xen/arch/x86/x86_64/compat/entry.S
>>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>>> @@ -197,7 +197,7 @@ ENTRY(cstar_enter)
>>> /* sti could live here when we don't switch page tables below. */
>>> CR4_PV32_RESTORE
>>> movq 8(%rsp),%rax /* Restore %rax. */
>>> - movq $FLAT_KERNEL_SS,8(%rsp)
>>> + movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
>>> pushq %r11
>>> pushq $FLAT_USER_CS32
>>> pushq %rcx
>>> @@ -223,6 +223,11 @@ ENTRY(cstar_enter)
>>> movq VCPU_domain(%rbx),%rcx
>>> cmpb $0,DOMAIN_is_32bit_pv(%rcx)
>>> je switch_to_kernel
>>> +
>>> + /* Fix up reported %cs/%ss for compat domains. */
>>> + movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
>>> + movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
>> I wonder if it would be better to introduce COMPAT_FLAT_RING3_* in
>> xen-x86_64.h?
>>
>> In any case, the reasoning and code looks correct to me:
>>
>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>
> I considered that, and am open to suggestions. The problem is that we
> don't want to put COMPAT definitions in that header file, because they
> are inapplicable to 64bit PV guests, who are intended consumers.
Add a way to include x86_32.h defining the needed COMPAT_* macros only?
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
2018-04-09 10:49 ` Juergen Gross
@ 2018-04-09 12:01 ` Andrew Cooper
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2018-04-09 12:01 UTC (permalink / raw)
To: Juergen Gross, Wei Liu; +Cc: Roger Pau Monné, Jan Beulich, Xen-devel
On 09/04/18 11:49, Juergen Gross wrote:
> On 09/04/18 12:46, Andrew Cooper wrote:
>> On 09/04/18 11:44, Wei Liu wrote:
>>> On Mon, Apr 09, 2018 at 10:44:47AM +0100, Andrew Cooper wrote:
>>>> The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
>>>> wrong constant to use. Switch to FLAT_USER_SS32.
>>>>
>>>> For compat domains however, the reported values are entirely bogus.
>>>> FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
>>>> FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
>>>>
>>>> The guests SYSCALL callback is invoked with a broken iret frame, and if left
>>>> unmodified by the guest, will fail on the way back out when Xen's iret tries
>>>> to load a code segment into %ss.
>>>>
>>>> In practice, this is only a problem for 32bit PV guests on AMD hardware, as
>>>> Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
>>>>
>>>> This appears to have been broken ever since 64bit support was added to Xen,
>>>> and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>> CC: Juergen Gross <jgross@suse.com>
>>>>
>>>> This wants backporting basically everywhere, and as such, also wants to be
>>>> considered for 4.11 at this point.
>>>> ---
>>>> xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
>>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
>>>> index 6c7fcf9..2bc046c 100644
>>>> --- a/xen/arch/x86/x86_64/compat/entry.S
>>>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>>>> @@ -197,7 +197,7 @@ ENTRY(cstar_enter)
>>>> /* sti could live here when we don't switch page tables below. */
>>>> CR4_PV32_RESTORE
>>>> movq 8(%rsp),%rax /* Restore %rax. */
>>>> - movq $FLAT_KERNEL_SS,8(%rsp)
>>>> + movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
>>>> pushq %r11
>>>> pushq $FLAT_USER_CS32
>>>> pushq %rcx
>>>> @@ -223,6 +223,11 @@ ENTRY(cstar_enter)
>>>> movq VCPU_domain(%rbx),%rcx
>>>> cmpb $0,DOMAIN_is_32bit_pv(%rcx)
>>>> je switch_to_kernel
>>>> +
>>>> + /* Fix up reported %cs/%ss for compat domains. */
>>>> + movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
>>>> + movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
>>> I wonder if it would be better to introduce COMPAT_FLAT_RING3_* in
>>> xen-x86_64.h?
>>>
>>> In any case, the reasoning and code looks correct to me:
>>>
>>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>> I considered that, and am open to suggestions. The problem is that we
>> don't want to put COMPAT definitions in that header file, because they
>> are inapplicable to 64bit PV guests, who are intended consumers.
> Add a way to include x86_32.h defining the needed COMPAT_* macros only?
For the same argument as the 64bit side, x86_32.h shouldn't contain
COMPAT_* defines, as they are inapplicable.
Furthermore, the constants can't be shared in such an include, because
of naming conflicts with x86_64.h.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
2018-04-09 10:44 ` Wei Liu
2018-04-09 10:46 ` Andrew Cooper
2018-04-09 10:47 ` Juergen Gross
@ 2018-04-09 12:02 ` Jan Beulich
2018-04-09 12:03 ` Andrew Cooper
2 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2018-04-09 12:02 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu; +Cc: Juergen Gross, Xen-devel, Roger Pau Monné
>>> On 09.04.18 at 12:44, <wei.liu2@citrix.com> wrote:
> On Mon, Apr 09, 2018 at 10:44:47AM +0100, Andrew Cooper wrote:
>> The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
>> wrong constant to use. Switch to FLAT_USER_SS32.
>>
>> For compat domains however, the reported values are entirely bogus.
>> FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
>> FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
>>
>> The guests SYSCALL callback is invoked with a broken iret frame, and if left
>> unmodified by the guest, will fail on the way back out when Xen's iret tries
>> to load a code segment into %ss.
>>
>> In practice, this is only a problem for 32bit PV guests on AMD hardware, as
>> Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
>>
>> This appears to have been broken ever since 64bit support was added to Xen,
>> and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>>
>> This wants backporting basically everywhere, and as such, also wants to be
>> considered for 4.11 at this point.
>> ---
>> xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/x86_64/compat/entry.S
> b/xen/arch/x86/x86_64/compat/entry.S
>> index 6c7fcf9..2bc046c 100644
>> --- a/xen/arch/x86/x86_64/compat/entry.S
>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>> @@ -197,7 +197,7 @@ ENTRY(cstar_enter)
>> /* sti could live here when we don't switch page tables below. */
>> CR4_PV32_RESTORE
>> movq 8(%rsp),%rax /* Restore %rax. */
>> - movq $FLAT_KERNEL_SS,8(%rsp)
>> + movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
>> pushq %r11
>> pushq $FLAT_USER_CS32
>> pushq %rcx
>> @@ -223,6 +223,11 @@ ENTRY(cstar_enter)
>> movq VCPU_domain(%rbx),%rcx
>> cmpb $0,DOMAIN_is_32bit_pv(%rcx)
>> je switch_to_kernel
>> +
>> + /* Fix up reported %cs/%ss for compat domains. */
>> + movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
>> + movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
>
> I wonder if it would be better to introduce COMPAT_FLAT_RING3_* in
> xen-x86_64.h?
We already have FLAT_COMPAT_RING3_{CS,DS,SS} - I don't see
why two of them couldn't be used here (or their FLAT_COMPAT_USER_*
aliases). With that
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry
2018-04-09 12:02 ` Jan Beulich
@ 2018-04-09 12:03 ` Andrew Cooper
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Cooper @ 2018-04-09 12:03 UTC (permalink / raw)
To: Jan Beulich, Wei Liu; +Cc: Juergen Gross, Xen-devel, Roger Pau Monné
On 09/04/18 13:02, Jan Beulich wrote:
>>>> On 09.04.18 at 12:44, <wei.liu2@citrix.com> wrote:
>> On Mon, Apr 09, 2018 at 10:44:47AM +0100, Andrew Cooper wrote:
>>> The existing FLAT_KERNEL_SS expands to the correct value, 0xe02b, but is the
>>> wrong constant to use. Switch to FLAT_USER_SS32.
>>>
>>> For compat domains however, the reported values are entirely bogus.
>>> FLAT_USER_SS32 (value 0xe02b) is FLAT_RING3_CS in the 32bit ABI, while
>>> FLAT_USER_CS32 (value 0xe023) is FLAT_RING1_DS with an RPL of 3.
>>>
>>> The guests SYSCALL callback is invoked with a broken iret frame, and if left
>>> unmodified by the guest, will fail on the way back out when Xen's iret tries
>>> to load a code segment into %ss.
>>>
>>> In practice, this is only a problem for 32bit PV guests on AMD hardware, as
>>> Intel hardware doesn't permit the SYSCALL instruction outside of 64bit mode.
>>>
>>> This appears to have been broken ever since 64bit support was added to Xen,
>>> and has gone unnoticed because Linux doesn't use SYSCALL in 32bit builds.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>> CC: Wei Liu <wei.liu2@citrix.com>
>>> CC: Juergen Gross <jgross@suse.com>
>>>
>>> This wants backporting basically everywhere, and as such, also wants to be
>>> considered for 4.11 at this point.
>>> ---
>>> xen/arch/x86/x86_64/compat/entry.S | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/x86_64/compat/entry.S
>> b/xen/arch/x86/x86_64/compat/entry.S
>>> index 6c7fcf9..2bc046c 100644
>>> --- a/xen/arch/x86/x86_64/compat/entry.S
>>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>>> @@ -197,7 +197,7 @@ ENTRY(cstar_enter)
>>> /* sti could live here when we don't switch page tables below. */
>>> CR4_PV32_RESTORE
>>> movq 8(%rsp),%rax /* Restore %rax. */
>>> - movq $FLAT_KERNEL_SS,8(%rsp)
>>> + movq $FLAT_USER_SS32, 8(%rsp) /* Assume a 64bit domain. Compat handled lower. */
>>> pushq %r11
>>> pushq $FLAT_USER_CS32
>>> pushq %rcx
>>> @@ -223,6 +223,11 @@ ENTRY(cstar_enter)
>>> movq VCPU_domain(%rbx),%rcx
>>> cmpb $0,DOMAIN_is_32bit_pv(%rcx)
>>> je switch_to_kernel
>>> +
>>> + /* Fix up reported %cs/%ss for compat domains. */
>>> + movl $0xe033, UREGS_ss(%rsp) /* Compat FLAT_RING3_SS */
>>> + movl $0xe02b, UREGS_cs(%rsp) /* Compat FLAT_RING3_CS */
>> I wonder if it would be better to introduce COMPAT_FLAT_RING3_* in
>> xen-x86_64.h?
> We already have FLAT_COMPAT_RING3_{CS,DS,SS} - I don't see
> why two of them couldn't be used here (or their FLAT_COMPAT_USER_*
> aliases). With that
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Ooh - so we have. I'd completely missed those. Will fix.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-09 12:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-09 9:44 [PATCH for-4.11] x86/pv: Fix up erroneous segments for 32bit syscall entry Andrew Cooper
2018-04-09 10:44 ` Wei Liu
2018-04-09 10:46 ` Andrew Cooper
2018-04-09 10:49 ` Juergen Gross
2018-04-09 12:01 ` Andrew Cooper
2018-04-09 10:47 ` Juergen Gross
2018-04-09 12:02 ` Jan Beulich
2018-04-09 12:03 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.