Re: [Xen-devel] [PATCH 1/3] x86: relax GDT check in arch_set_info_guest()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.12.2019 20:51, Andrew Cooper wrote:
> On 06/12/2019 11:32, Jan Beulich wrote:
>> On 06.12.2019 11:25, Andrew Cooper wrote:
>>> On 06/12/2019 10:14, Jan Beulich wrote:
>>>> It is wrong for us to check frames beyond the guest specified limit.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> I don't completely agree.  The code has been like this since it was
>>> introduced, and is used to check data from the domain builder (inc
>>> migration), and from the guests.
>>>
>>> At the moment, every caller is required not to pass junk in unused
>>> frames, and I don't see an issue with keeping this behaviour.
>> Keeping the behavior isn't going to break anything, yes, but it
>> shouldn't have been this way to begin with. I simply don't see
>> the value of validating data we're not consuming anyway. Perhaps
>> I could say "not helpful" or "pointless" instead of "wrong" ...
> 
> But in other cases we go out of our way to check parameters (especially
> reserved fields) even when they aren't presently consumed.

Which we do to make sure we could use the fields down the road
without breaking existing callers. That's quite different from
the overzealous checking we do here.

> i.e. what do we gain (other than more complicated code) by relaxing a
> restriction we know is obeyed by every caller?

First - I don't think the code gets more complicated by this
change (nor the LDT counterpart). If anything I'm seeing a
really minor simplification (by consistently using a now
common variable). Further, if you look closely, you'll note
that the compat path is already only checking the specified
number of frames. Hence I'm bringing the non-compat one in
line, i.e. an improvement in consistency.

Jan

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