On 15/01/2021 11:43, Jan Beulich wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -4013,6 +4013,59 @@ 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;
> It doesn't look like this initializer was needed. The only
> use of i that I can spot is the loop near the end, which
> starts from 0.
Its possibly stale from v1. I'll adjust.
>> + mfn_t tmp;
>> + void **vaddrs;
>> + int rc;
>> +
>> + /* Overflow checks */
>> + if ( frame + nr_frames < frame )
>> + return -EINVAL;
>> +
>> + tot_frames = frame + nr_frames;
>> + if ( tot_frames != frame + nr_frames )
>> + return -EINVAL;
> Can't these two be folded into
>
> unsigned int tot_frames = frame + nr_frames;
>
> if ( tot_frames < frame )
> return -EINVAL;
>
> ? Both truncation and wrapping look to be taken care of this
> way.
Not when frame is a multiple of 4G (or fractionally over, I think).
This ABI with mismatched widths really is obnoxious to work with.
~Andrew