Re: [Xen-devel] [PATCH] x86/xpti: Hide almost all of .text and all .data/.rodata/.bss mappings

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.02.18 at 13:34, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 14/02/18 09:14, Jan Beulich wrote:
>>>>> On 13.02.18 at 20:45, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> @@ -651,9 +654,6 @@ static int clone_mapping(const void *ptr, 
>>> root_pgentry_t *rpt)
>>>      l2_pgentry_t *pl2e;
>>>      l1_pgentry_t *pl1e;
>>>  
>>> -    if ( linear < DIRECTMAP_VIRT_START )
>>> -        return 0;
>> Isn't outright removing this going a little too far?
> 
> Considering it is buggy, no.  Retuning success after doing nothing is
> very antisocial.
> 
> If anything, it should return -EINVAL so the caller knows that something
> expected happened, but it should also reject attempts to clone mappings
> beyond the end of the directmap (as those will explode when we switch to
> PV context), and the XEN_VIRT_* range needs white-listing as ok to clone
> for this usecase to work.

Returning -EINVAL would have been wrong without excluding the
XEN_VIRT_* range - the boot CPU has at least its IDT there.
Tightening the check would be fine with me.

>>> --- a/xen/arch/x86/x86_64/compat/entry.S
>>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>>> @@ -13,6 +13,8 @@
>>>  #include <public/xen.h>
>>>  #include <irq_vectors.h>
>>>  
>>> +        .section .text.entry, "ax", @progbits
>>> +
>>>  ENTRY(entry_int82)
>>>          ASM_CLAC
>>>          pushq $0
>> This also puts compat_create_bounce_frame into the entry section,
>> which surely isn't needed. Same for the non-compat variant.
> 
> I considered that.  Excluding those ranges won't reduce the number of
> allocations we make, but it does complicate backports.

Complicate backports? The context of the section directive may
change some, but that's not a significant complication imo.

Jan


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