Re: [PATCH 2/3] xen/common: llc-coloring: Fix off-by-one in parse_color_config()

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 09/04/2026 13:47, Andrew Cooper wrote:
> On 09/04/2026 12:39 pm, Michal Orzel wrote:
>> The check uses >= to compare the total number of colors against
>> max_num_colors (which is ARRAY_SIZE of the colors array).  This
>> incorrectly rejects input that would exactly fill the array.
>>
>> For example, with NR_LLC_COLORS=16, specifying 1 color for Xen and 15
>> for dom0 would fail.
>>
>> Change >= to > so that exactly filling the array is permitted.
>>
>> Fixes: 95ef5ddf8a ("xen/arm: add Dom0 cache coloring support")
>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
>> ---
>>  xen/common/llc-coloring.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/common/llc-coloring.c b/xen/common/llc-coloring.c
>> index eb7c72b24023..30c1594dac9f 100644
>> --- a/xen/common/llc-coloring.c
>> +++ b/xen/common/llc-coloring.c
>> @@ -78,7 +78,7 @@ static int __init parse_color_config(const char *buf, 
>> unsigned int colors[],
>>  
>>          if ( end >= NR_LLC_COLORS || start > end ||
>>               (end - start) >= (UINT_MAX - *num_colors) ||
>> -             (*num_colors + (end - start + 1)) >= max_num_colors )
>> +             (*num_colors + (end - start + 1)) > max_num_colors )
>>              return -EINVAL;
>>  
>>          /* Colors are range checked in check_colors() */
> 
> This boundary was changed by
> https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=cba8a584de171c8c4510709c2edc9f1cf86b21ab
> because it was off-by-one.
> 
> Are you saying that the analysis in that patch was wrong?
I examined the scenario that is a default for dom0 i.e. dom0 gets all the colors
by default. This is equivalent to setting dom0-llc-colors=0-15. If I set this, I
will get a message:
(XEN) parameter "dom0-llc-colors" has invalid value "0-15", rc=-22!

I admit that I added wrong example in commit msg.

~Michal

> 
> ~Andrew