Re: [PATCH 2/5] xen/gnttab: Rework resource acquisition

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/09/2020 14:34, Jan Beulich wrote:
> On 22.09.2020 15:10, Andrew Cooper wrote:
>> On 29/07/2020 21:02, Jan Beulich wrote:
>>> On 28.07.2020 13:37, Andrew Cooper wrote:
>>>> --- a/xen/common/grant_table.c
>>>> +++ b/xen/common/grant_table.c
>>>> @@ -4013,6 +4013,72 @@ static int gnttab_get_shared_frame_mfn(struct
>>>> domain *d,
>>>>       return 0;
>>>>   }
>>>>   +int gnttab_acquire_resource(
>>>> +    struct domain *d, unsigned int id, unsigned long frame,
>>>> +    unsigned int nr_frames, xen_pfn_t mfn_list[])
>>>> +{
>>>> +    struct grant_table *gt = d->grant_table;
>>>> +    unsigned int i = nr_frames, tot_frames;
>>>> +    void **vaddrs;
>>>> +    int rc = 0;
>>>> +
>>>> +    /* Input sanity. */
>>>> +    if ( !nr_frames )
>>>> +        return -EINVAL;
>>> I can't seem to be able to find an equivalent of this in the old logic,
>>> and hence this looks like an unwarranted change in behavior to me. We
>>> have quite a few hypercall ops where some count being zero is simply
>>> a no-op, i.e. yielding success without doing anything.
>> There is no possible circumstance when getting here requesting 0 is
>> legitimate.
>>
>> Tolerating a zero input for a mapping request is a very expensive way of
>> hiding caller bugs.
> That's just one possible view.

... that is fully enforced in the ecosystem we work in.

You can't get a 0 to here in the first place.

However, when it comes to the XLAT and the chunking fixes, getting 0
here is a hard error, and I'm not interested in breaking the that logic
for a non-existent corner case.

~Andrew