Re: [PATCH v6 05/15] xen: extend domctl interface for cache coloring

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.02.2024 12:46, Carlo Nonato wrote:
> On Thu, Feb 1, 2024 at 2:51 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 29.01.2024 18:18, Carlo Nonato wrote:
>>> @@ -229,6 +230,30 @@ int __init dom0_set_llc_colors(struct domain *d)
>>>      return domain_check_colors(d);
>>>  }
>>>
>>> +int domain_set_llc_colors_domctl(struct domain *d,
>>> +                                 const struct xen_domctl_set_llc_colors 
>>> *config)
>>
>> What purpose has the "domctl" in the function name?
>>
>>> +{
>>> +    unsigned int *colors;
>>> +
>>> +    if ( d->num_llc_colors )
>>> +        return -EEXIST;
>>> +
>>> +    if ( !config->num_llc_colors )
>>> +        return domain_set_default_colors(d);
>>> +
>>> +    colors = alloc_colors(config->num_llc_colors);
>>> +    if ( !colors )
>>> +        return -ENOMEM;
>>
>> Hmm, I see here you call the function without first having bounds checked
>> the input. But in case of too big a value, -ENOMEM is inappropriate, so
>> such a check wants adding up front anyway.
>>
>>> +    if ( copy_from_guest(colors, config->llc_colors, 
>>> config->num_llc_colors) )
>>> +        return -EFAULT;
>>
>> There again wants to be a check that the pointed to values are the same,
>> or at least of the same size. Else you'd need to do element-wise copy.
> 
> Sorry to bring this back again, but I've just noticed copy_from_guest()
> already checks for type compatibility. For what regards the size I don't think
> I understood what to check. colors is defined to be of the same size of
> config->llc_colors.

Oh, you're right. But the other case was a memcpy(), wasn't it?

Jan