Re: [Xen-devel] [PATCH v2] x86/stackframe/32: repair 32-bit Xen PV

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.11.2019 23:44, Thomas Gleixner wrote:
> On Tue, 29 Oct 2019, Jan Beulich wrote:
> 
>> Once again RPL checks have been introduced which don't account for a
>> 32-bit kernel living in ring 1 when running in a PV Xen domain.
>>
>> The case in FIXUP_FRAME has been preventing boot; adjust BUG_IF_WRONG_CR3
>> as well just in case.
> 
> Either it's required and correct or it's not. Just in case is not helpful
> at all.

_If_ this macro sits on any path reachable when running PV under
Xen, then it's wrong. If any such use gets added down the road,
then it's latently wrong, which is bad enough imo, and hence
warrants the change even without analyzing whether there's
already an affected path.

>> Fixes: 3c88c692c287 ("x86/stackframe/32: Provide consistent pt_regs")
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
>> --- a/arch/x86/entry/entry_32.S
>> +++ b/arch/x86/entry/entry_32.S
>> @@ -48,6 +48,13 @@
>>  
>>  #include "calling.h"
>>  
>> +/*
>> + * When running on Xen PV, the actual %cs register's RPL in the kernel is 1,
>> + * not 0. If we need to distinguish between a %cs from kernel mode and a %cs
>> + * from user mode, we can do test $2 instead of test $3.
>> + */
>> +#define USER_SEGMENT_RPL_MASK 2
> 
> No. The define want's to be right next to the SEGMENT_RPL_MASK define
> including a less ASM focussed comment like this:
> 
> /*
>  * When running on Xen PV, the actual priviledge level of the kernel is 1,
>  * not 0. Testing the Requested Priviledge Level in a segment selector to
>  * determine whether the context is user mode or kernel mode with
>  * SEGMENT_RPL_MASK is wrong because the PV kernels priviledge level
>  * matches the 0x03 mask.
>  *
>  * Testing with USER_SEGMENT_RPL_MASK is valid for both native and Xen PV
>  * kernels because Priviledge Level 2 is never used.
>  */
> 
> Hmm?

I simply used almost exactly what Andy had suggested as a comment. He
also didn't object to the definition sitting here (it's not needed
after all outside of this file). Can the two of you please reach
agreement, for me to act upon?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel