update_runstate_area and Linux KPTI

update_runstate_area and Linux KPTI

[Julien Grall]

Hi,

While I was looking at some unrelated problem with Xen ARM P2M code, I 
noticed that the function update_runstate_area is using guest virtual 
address to update the vCPU runstate. That function will be called when 
context switch to a vCPU. However, that vCPU may run in userspace 
context. When KPTI (kernel page table isolation) is used,

In the best case, that address is not mapped into the page-table 
currently used. Xen will not be able to update the region.

In the worst case, that address is mapped to a different region and Xen 
will corrupt some bits of the memory.

The code looks fundamentally wrong on Arm, I am entirely not sure about x86.

It look like to me that Xen should always use the guest physical address 
and therefore translate the virtual address to a physical one in 
VCPUOP_register_runstate_memory_area. So only the physical address will 
be used in update_runstate_area making the function much safer.

Any opinion on this approach?

Cheers,

-- 
Julien Grall

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

Re: update_runstate_area and Linux KPTI

[Andrew Cooper]

On 02/03/18 15:57, Julien Grall wrote:
> Hi,
>
> While I was looking at some unrelated problem with Xen ARM P2M code, I
> noticed that the function update_runstate_area is using guest virtual
> address to update the vCPU runstate. That function will be called when
> context switch to a vCPU. However, that vCPU may run in userspace
> context. When KPTI (kernel page table isolation) is used,
>
> In the best case, that address is not mapped into the page-table
> currently used. Xen will not be able to update the region.
>
> In the worst case, that address is mapped to a different region and
> Xen will corrupt some bits of the memory.
>
> The code looks fundamentally wrong on Arm, I am entirely not sure
> about x86.
>
> It look like to me that Xen should always use the guest physical
> address and therefore translate the virtual address to a physical one
> in VCPUOP_register_runstate_memory_area. So only the physical address
> will be used in update_runstate_area making the function much safer.
>
> Any opinion on this approach?

All the Xen interfaces like this built upon linear (virtual) addresses
are fundamentally wrong, but that horse has bolted.

On the x86 side, we've got a gross hack where we try and ignore
pagefaults, leaving a note to come back and try again later.  It gets
even more complicated with SMAP (PAN on ARM, iirc).

The proper way to do this is indeed by a nominated (guest) physical
address, at which point Xen can make all/any updates at times of its
choosing, and the guests pagetable/permissions state at an instantaneous
moment don't matter.

If you've got time to do this, then please do.  It will be a definite
improvement.

~Andrew

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

Re: update_runstate_area and Linux KPTI

[Jan Beulich]

>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
> The proper way to do this is indeed by a nominated (guest) physical
> address, at which point Xen can make all/any updates at times of its
> choosing, and the guests pagetable/permissions state at an instantaneous
> moment don't matter.
> 
> If you've got time to do this, then please do.  It will be a definite
> improvement.

Just to be avoid unnecessary effort in the wrong direction: I don't
think you can alter the current interface. You'd have to add a new
one, and we could then deprecate (but never abandon) the current
one.

Jan


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

Re: update_runstate_area and Linux KPTI

[Andrew Cooper]

On 02/03/18 16:18, Jan Beulich wrote:
>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>> The proper way to do this is indeed by a nominated (guest) physical
>> address, at which point Xen can make all/any updates at times of its
>> choosing, and the guests pagetable/permissions state at an instantaneous
>> moment don't matter.
>>
>> If you've got time to do this, then please do.  It will be a definite
>> improvement.
> Just to be avoid unnecessary effort in the wrong direction: I don't
> think you can alter the current interface. You'd have to add a new
> one, and we could then deprecate (but never abandon) the current
> one.

No - we sadly can't remove the current interface (at least for a long
while), but we can immediately deprecate it when a better alternative is
available.

OTOH, I think it would be a very good idea to have a Kconfig option so
we can selectively excise legacy interfaces.  I expect this will be of
particular interest to embedded/bespoke configurations.

~Andrew

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

Re: update_runstate_area and Linux KPTI

[Julien Grall]

On 02/03/18 16:18, Jan Beulich wrote:
>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>> The proper way to do this is indeed by a nominated (guest) physical
>> address, at which point Xen can make all/any updates at times of its
>> choosing, and the guests pagetable/permissions state at an instantaneous
>> moment don't matter.
>>
>> If you've got time to do this, then please do.  It will be a definite
>> improvement.
> 
> Just to be avoid unnecessary effort in the wrong direction: I don't
> think you can alter the current interface. You'd have to add a new
> one, and we could then deprecate (but never abandon) the current
> one.

I was only planning to store the guest physical address rather than the 
virtual address as we do today. Is that considered as an alteration of 
the current interface?

In other words, the current version (e.g store virtual address) is just 
broken and going to be worst with KPTI kernel. I can't see how this 
could ever work properly on OS with different set of page-tables.

Cheers,

-- 
Julien Grall

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

Re: update_runstate_area and Linux KPTI

[Jan Beulich]

>>> On 02.03.18 at 17:25, <julien.grall@arm.com> wrote:
> On 02/03/18 16:18, Jan Beulich wrote:
>>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>>> The proper way to do this is indeed by a nominated (guest) physical
>>> address, at which point Xen can make all/any updates at times of its
>>> choosing, and the guests pagetable/permissions state at an instantaneous
>>> moment don't matter.
>>>
>>> If you've got time to do this, then please do.  It will be a definite
>>> improvement.
>> 
>> Just to be avoid unnecessary effort in the wrong direction: I don't
>> think you can alter the current interface. You'd have to add a new
>> one, and we could then deprecate (but never abandon) the current
>> one.
> 
> I was only planning to store the guest physical address rather than the 
> virtual address as we do today. Is that considered as an alteration of 
> the current interface?

Yes, it is, as an existing PV kernel could deliberately alter the
mappings underlying the linear address it has handed us.

Jan


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

Re: update_runstate_area and Linux KPTI

[Juergen Gross]

On 02/03/18 17:25, Julien Grall wrote:
> 
> 
> On 02/03/18 16:18, Jan Beulich wrote:
>>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>>> The proper way to do this is indeed by a nominated (guest) physical
>>> address, at which point Xen can make all/any updates at times of its
>>> choosing, and the guests pagetable/permissions state at an instantaneous
>>> moment don't matter.
>>>
>>> If you've got time to do this, then please do.  It will be a definite
>>> improvement.
>>
>> Just to be avoid unnecessary effort in the wrong direction: I don't
>> think you can alter the current interface. You'd have to add a new
>> one, and we could then deprecate (but never abandon) the current
>> one.
> 
> I was only planning to store the guest physical address rather than the
> virtual address as we do today. Is that considered as an alteration of
> the current interface?

I don't think so. It should be perfectly fine to assume the mapping of
the registered virtual address isn't changed by the guest.

> In other words, the current version (e.g store virtual address) is just
> broken and going to be worst with KPTI kernel. I can't see how this
> could ever work properly on OS with different set of page-tables.

map_vcpu_info() seems to be a nice example how this should be done.
This should make update_runstate_area() simpler and faster.


Juergen

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

Re: update_runstate_area and Linux KPTI

[Juergen Gross]

On 02/03/18 17:51, Jan Beulich wrote:
>>>> On 02.03.18 at 17:25, <julien.grall@arm.com> wrote:
>> On 02/03/18 16:18, Jan Beulich wrote:
>>>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>>>> The proper way to do this is indeed by a nominated (guest) physical
>>>> address, at which point Xen can make all/any updates at times of its
>>>> choosing, and the guests pagetable/permissions state at an instantaneous
>>>> moment don't matter.
>>>>
>>>> If you've got time to do this, then please do.  It will be a definite
>>>> improvement.
>>>
>>> Just to be avoid unnecessary effort in the wrong direction: I don't
>>> think you can alter the current interface. You'd have to add a new
>>> one, and we could then deprecate (but never abandon) the current
>>> one.
>>
>> I was only planning to store the guest physical address rather than the 
>> virtual address as we do today. Is that considered as an alteration of 
>> the current interface?
> 
> Yes, it is, as an existing PV kernel could deliberately alter the
> mappings underlying the linear address it has handed us.

Linux pvops kernel isn't doing this. Mini-OS neither. I guess kernel-xen
would be okay with this, too. And I bet BSD is also fine.

Seriously: any kernel playing such tricks is asking for problems.

We shouldn't support operation modes which make no sense just for the
sake of compatibility, IMO.


Juergen

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

Re: update_runstate_area and Linux KPTI

[Andrew Cooper]

On 02/03/18 17:05, Juergen Gross wrote:
> On 02/03/18 17:51, Jan Beulich wrote:
>>>>> On 02.03.18 at 17:25, <julien.grall@arm.com> wrote:
>>> On 02/03/18 16:18, Jan Beulich wrote:
>>>>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>>>>> The proper way to do this is indeed by a nominated (guest) physical
>>>>> address, at which point Xen can make all/any updates at times of its
>>>>> choosing, and the guests pagetable/permissions state at an instantaneous
>>>>> moment don't matter.
>>>>>
>>>>> If you've got time to do this, then please do.  It will be a definite
>>>>> improvement.
>>>> Just to be avoid unnecessary effort in the wrong direction: I don't
>>>> think you can alter the current interface. You'd have to add a new
>>>> one, and we could then deprecate (but never abandon) the current
>>>> one.
>>> I was only planning to store the guest physical address rather than the 
>>> virtual address as we do today. Is that considered as an alteration of 
>>> the current interface?
>> Yes, it is, as an existing PV kernel could deliberately alter the
>> mappings underlying the linear address it has handed us.
> Linux pvops kernel isn't doing this. Mini-OS neither. I guess kernel-xen
> would be okay with this, too. And I bet BSD is also fine.
>
> Seriously: any kernel playing such tricks is asking for problems.
>
> We shouldn't support operation modes which make no sense just for the
> sake of compatibility, IMO.

I'd love to do this, but we cant.  Older Linux used to have a virtual
buffer spanning a page boundary.  Changing the behaviour under that will
cause older setups to explode.

~Andrew

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

Re: update_runstate_area and Linux KPTI

[Juergen Gross]

On 02/03/18 18:09, Andrew Cooper wrote:
> On 02/03/18 17:05, Juergen Gross wrote:
>> On 02/03/18 17:51, Jan Beulich wrote:
>>>>>> On 02.03.18 at 17:25, <julien.grall@arm.com> wrote:
>>>> On 02/03/18 16:18, Jan Beulich wrote:
>>>>>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>>>>>> The proper way to do this is indeed by a nominated (guest) physical
>>>>>> address, at which point Xen can make all/any updates at times of its
>>>>>> choosing, and the guests pagetable/permissions state at an instantaneous
>>>>>> moment don't matter.
>>>>>>
>>>>>> If you've got time to do this, then please do.  It will be a definite
>>>>>> improvement.
>>>>> Just to be avoid unnecessary effort in the wrong direction: I don't
>>>>> think you can alter the current interface. You'd have to add a new
>>>>> one, and we could then deprecate (but never abandon) the current
>>>>> one.
>>>> I was only planning to store the guest physical address rather than the 
>>>> virtual address as we do today. Is that considered as an alteration of 
>>>> the current interface?
>>> Yes, it is, as an existing PV kernel could deliberately alter the
>>> mappings underlying the linear address it has handed us.
>> Linux pvops kernel isn't doing this. Mini-OS neither. I guess kernel-xen
>> would be okay with this, too. And I bet BSD is also fine.
>>
>> Seriously: any kernel playing such tricks is asking for problems.
>>
>> We shouldn't support operation modes which make no sense just for the
>> sake of compatibility, IMO.
> 
> I'd love to do this, but we cant.  Older Linux used to have a virtual
> buffer spanning a page boundary.  Changing the behaviour under that will
> cause older setups to explode.

Adding a special per-domain mapping for that purpose would work.


Juergen


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

Re: update_runstate_area and Linux KPTI

[Julien Grall]

Hi Juergen,

On 03/02/2018 05:25 PM, Juergen Gross wrote:
> On 02/03/18 18:09, Andrew Cooper wrote:
>> On 02/03/18 17:05, Juergen Gross wrote:
>>> On 02/03/18 17:51, Jan Beulich wrote:
>>>>>>> On 02.03.18 at 17:25, <julien.grall@arm.com> wrote:
>>>>> On 02/03/18 16:18, Jan Beulich wrote:
>>>>>>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>>>>>>> The proper way to do this is indeed by a nominated (guest) physical
>>>>>>> address, at which point Xen can make all/any updates at times of its
>>>>>>> choosing, and the guests pagetable/permissions state at an instantaneous
>>>>>>> moment don't matter.
>>>>>>>
>>>>>>> If you've got time to do this, then please do.  It will be a definite
>>>>>>> improvement.
>>>>>> Just to be avoid unnecessary effort in the wrong direction: I don't
>>>>>> think you can alter the current interface. You'd have to add a new
>>>>>> one, and we could then deprecate (but never abandon) the current
>>>>>> one.
>>>>> I was only planning to store the guest physical address rather than the
>>>>> virtual address as we do today. Is that considered as an alteration of
>>>>> the current interface?
>>>> Yes, it is, as an existing PV kernel could deliberately alter the
>>>> mappings underlying the linear address it has handed us.
>>> Linux pvops kernel isn't doing this. Mini-OS neither. I guess kernel-xen
>>> would be okay with this, too. And I bet BSD is also fine.
>>>
>>> Seriously: any kernel playing such tricks is asking for problems.
>>>
>>> We shouldn't support operation modes which make no sense just for the
>>> sake of compatibility, IMO.
>>
>> I'd love to do this, but we cant.  Older Linux used to have a virtual
>> buffer spanning a page boundary.  Changing the behaviour under that will
>> cause older setups to explode.
> 
> Adding a special per-domain mapping for that purpose would work.

I am not sure to understand your suggestion here. Would you mind giving 
a bit more details?

If the buffer is spanning a page boundary (it seems to be the case on 
current Linux), you would need to map 2 pages using vmap in Xen 
per-VCPU. Would that be acceptable?

Cheers,

-- 
Julien Grall

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

Re: update_runstate_area and Linux KPTI

[Julien Grall]

Hi Andrew,

On 03/02/2018 05:09 PM, Andrew Cooper wrote:
> On 02/03/18 17:05, Juergen Gross wrote:
>> On 02/03/18 17:51, Jan Beulich wrote:
>>>>>> On 02.03.18 at 17:25, <julien.grall@arm.com> wrote:
>>>> On 02/03/18 16:18, Jan Beulich wrote:
>>>>>>>> On 02.03.18 at 17:04, <andrew.cooper3@citrix.com> wrote:
>>>>>> The proper way to do this is indeed by a nominated (guest) physical
>>>>>> address, at which point Xen can make all/any updates at times of its
>>>>>> choosing, and the guests pagetable/permissions state at an instantaneous
>>>>>> moment don't matter.
>>>>>>
>>>>>> If you've got time to do this, then please do.  It will be a definite
>>>>>> improvement.
>>>>> Just to be avoid unnecessary effort in the wrong direction: I don't
>>>>> think you can alter the current interface. You'd have to add a new
>>>>> one, and we could then deprecate (but never abandon) the current
>>>>> one.
>>>> I was only planning to store the guest physical address rather than the
>>>> virtual address as we do today. Is that considered as an alteration of
>>>> the current interface?
>>> Yes, it is, as an existing PV kernel could deliberately alter the
>>> mappings underlying the linear address it has handed us.
>> Linux pvops kernel isn't doing this. Mini-OS neither. I guess kernel-xen
>> would be okay with this, too. And I bet BSD is also fine.
>>
>> Seriously: any kernel playing such tricks is asking for problems.
>>
>> We shouldn't support operation modes which make no sense just for the
>> sake of compatibility, IMO.
> 
> I'd love to do this, but we cant.  Older Linux used to have a virtual
> buffer spanning a page boundary.  Changing the behaviour under that will
> cause older setups to explode.

For confirmation, by older you mean any current Linux?  Looking at them 
the runstate is coming from the percpu region. I can't find in the code 
anything promising to have the runstate page-aligned. Did I miss anything?

Cheers,

-- 
Julien Grall

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