Re: [PATCH 02/17] x86: split __{get,put}_user() into "guest" and "unsafe" variants

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.02.2021 14:07, Roger Pau Monné wrote:
> On Fri, Feb 05, 2021 at 05:26:33PM +0100, Jan Beulich wrote:
>> On 05.02.2021 17:18, Roger Pau Monné wrote:
>>> On Fri, Feb 05, 2021 at 05:13:22PM +0100, Jan Beulich wrote:
>>>> On 05.02.2021 16:43, Roger Pau Monné wrote:
>>>>> On Thu, Jan 14, 2021 at 04:04:11PM +0100, Jan Beulich wrote:
>>>>>> The "guest" variants are intended to work with (potentially) fully guest
>>>>>> controlled addresses, while the "unsafe" variants are not.
>>>>>
>>>>> Just to clarify, both work against user addresses, but guest variants
>>>>> need to be more careful because the guest provided address can also be
>>>>> modified?
>>>>>
>>>>> I'm trying to understand the difference between "fully guest
>>>>> controlled" and "guest controlled".
>>>>
>>>> Not exactly, not. "unsafe" means access to anything which may
>>>> fault, guest controlled or not. do_invalid_op()'s reading of
>>>> the insn stream is a good example - the faulting insn there
>>>> isn't guest controlled at all, but we still want to be careful
>>>> when trying to read these bytes, as we don't want to fully
>>>> trust %rip there.
> 
> Oh, I see. It's possible that %rip points to an unmapped address
> there, and we need to be careful when reading, even if the value of
> %rip cannot be controlled by the guest and can legitimacy point to
> Xen's address space.
> 
>>> Would it make sense to threat everything as 'guest' accesses for the
>>> sake of not having this difference?
>>
>> That's what we've been doing until now. It is the purpose of
>> this change to allow the two to behave differently.
>>
>>> I think having two accessors it's likely to cause confusion and could
>>> possibly lead to the wrong one being used in unexpected contexts. Does
>>> it add a too big performance penalty to always use the most
>>> restrictive one?
>>
>> The problem is the most restrictive one is going to be too
>> restrictive - we wouldn't be able to access Xen space anymore
>> e.g. from the place pointed at above as example. This is
>> because for guest accesses (but not for "unsafe" ones) we're
>> going to divert them into non-canonical space (and hence make
>> speculation impossible, as such an access would fault) if it
>> would touch Xen space.
> 
> Yes, I understand now. I think it would have been helpful (for me) to
> have the first sentence as:
> 
> The "guest" variants are intended to work with (potentially) fully guest
> controlled addresses, while the "unsafe" variants are expected to be
> used in order to access addresses not under the guest control, but
> that could trigger faults anyway (like accessing the instruction
> stream in do_invalid_op).

I can use some of this, but in particular "access addresses not
under the guest control" isn't entirely correct. The question isn't
whether there's a guest control aspect, but which part of the
address space the addresses are in. See specifically x86/PV: use
get_unsafe() instead of copy_from_unsafe()" for two pretty good
examples. The address within the linear page tables are - in a
way at least - still somewhat guest controlled, but we're
deliberately accessing Xen virtual addresses there.

Jan