On 06.02.2024 12:46, Carlo Nonato wrote:
> On Thu, Feb 1, 2024 at 2:51 PM Jan Beulich <jbeul...@suse.com> 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
