We can easily avoid the risk of wrapping UINT_MAX <-> 0 by applying a
check against the compile-time-constant maximum number of colors.

Additionally the overflow checks suffered from an off-by-1, as the parsed
ranges are inclusive (e.g. end == start being possible, requiring 1 array
slot, while availability of 0 slots was checked in that case).

Fixes: 6cdea3444eaf ("xen/arm: add Dom0 cache coloring support")
Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/llc-coloring.c
+++ b/xen/common/llc-coloring.c
@@ -76,8 +76,9 @@ static int __init parse_color_config(con
         else                /* Single value */
             end = start;
 
-        if ( start > end || (end - start) > (UINT_MAX - *num_colors) ||
-             (*num_colors + (end - start)) >= max_num_colors )
+        if ( end >= NR_LLC_COLORS || start > end ||
+             (end - start) >= (UINT_MAX - *num_colors) ||
+             (*num_colors + (end - start + 1)) >= max_num_colors )
             return -EINVAL;
 
         /* Colors are range checked in check_colors() */

On Tue, Mar 24, 2026 at 6:37 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> We can easily avoid the risk of wrapping UINT_MAX <-> 0 by applying a
> check against the compile-time-constant maximum number of colors.
>
> Additionally the overflow checks suffered from an off-by-1, as the parsed
> ranges are inclusive (e.g. end == start being possible, requiring 1 array
> slot, while availability of 0 slots was checked in that case).
>
> Fixes: 6cdea3444eaf ("xen/arm: add Dom0 cache coloring support")
> Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>

For reference, I previously reported the UINT_MAX wraparound aspect here [1]
and later also here [2]. The off-by-1 in the inclusive-range accounting looks
like a separate issue.

Those threads also mention a few other related corner cases.


Best regards,
Mykola


[1] https://patchew.org/Xen/20241217170637.233097-1-carlo.nonato@minervasys.tech/20241217170637.233097-5-carlo.nonato@minervasys.tech/#db5d6a67-61d9-48d3-b6c4-213c1cbbda21@gmail.com
[2] https://lists.xen.org/archives/html/xen-devel/2026-01/msg00369.html

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/llc-coloring.c
> +++ b/xen/common/llc-coloring.c
> @@ -76,8 +76,9 @@ static int __init parse_color_config(con
>          else                /* Single value */
>              end = start;
>
> -        if ( start > end || (end - start) > (UINT_MAX - *num_colors) ||
> -             (*num_colors + (end - start)) >= max_num_colors )
> +        if ( end >= NR_LLC_COLORS || start > end ||
> +             (end - start) >= (UINT_MAX - *num_colors) ||
> +             (*num_colors + (end - start + 1)) >= max_num_colors )
>              return -EINVAL;
>
>          /* Colors are range checked in check_colors() */
>
>

On 24.03.2026 17:51, Mykola Kvach wrote:
> On Tue, Mar 24, 2026 at 6:37 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> We can easily avoid the risk of wrapping UINT_MAX <-> 0 by applying a
>> check against the compile-time-constant maximum number of colors.
>>
>> Additionally the overflow checks suffered from an off-by-1, as the parsed
>> ranges are inclusive (e.g. end == start being possible, requiring 1 array
>> slot, while availability of 0 slots was checked in that case).
>>
>> Fixes: 6cdea3444eaf ("xen/arm: add Dom0 cache coloring support")
>> Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
> 
> For reference, I previously reported the UINT_MAX wraparound aspect here [1]
> and later also here [2].

I've added another Reported-by, yet I wonder (in particular wrt [2]): Why did
you not simply send patches? They likely would long have gone in.

Jan

> The off-by-1 in the inclusive-range accounting looks
> like a separate issue.
> 
> Those threads also mention a few other related corner cases.
> 
> 
> Best regards,
> Mykola
> 
> 
> [1] https://patchew.org/Xen/20241217170637.233097-1-carlo.nonato@minervasys.tech/20241217170637.233097-5-carlo.nonato@minervasys.tech/#db5d6a67-61d9-48d3-b6c4-213c1cbbda21@gmail.com
> [2] https://lists.xen.org/archives/html/xen-devel/2026-01/msg00369.html

On Wed, Mar 25, 2026 at 8:14 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 24.03.2026 17:51, Mykola Kvach wrote:
> > On Tue, Mar 24, 2026 at 6:37 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> We can easily avoid the risk of wrapping UINT_MAX <-> 0 by applying a
> >> check against the compile-time-constant maximum number of colors.
> >>
> >> Additionally the overflow checks suffered from an off-by-1, as the parsed
> >> ranges are inclusive (e.g. end == start being possible, requiring 1 array
> >> slot, while availability of 0 slots was checked in that case).
> >>
> >> Fixes: 6cdea3444eaf ("xen/arm: add Dom0 cache coloring support")
> >> Reported-by: Kamil Frankowicz <kamil.frankowicz@cert.pl>
> >
> > For reference, I previously reported the UINT_MAX wraparound aspect here [1]
> > and later also here [2].
>
> I've added another Reported-by, yet I wonder (in particular wrt [2]): Why did
> you not simply send patches? They likely would long have gone in.

Thank you.

I did not have the time earlier to turn those reports into proper patches.

This month I have had some bandwidth again, so I started revisiting a number
of previously reported issues.

At the moment I am also preparing some other fixes related to ITS, and the
LLC issues were next on my list.


Best regards,
Mykola


>
> Jan
>
> > The off-by-1 in the inclusive-range accounting looks
> > like a separate issue.
> >
> > Those threads also mention a few other related corner cases.
> >
> >
> > Best regards,
> > Mykola
> >
> >
> > [1] https://patchew.org/Xen/20241217170637.233097-1-carlo.nonato@minervasys.tech/20241217170637.233097-5-carlo.nonato@minervasys.tech/#db5d6a67-61d9-48d3-b6c4-213c1cbbda21@gmail.com
> > [2] https://lists.xen.org/archives/html/xen-devel/2026-01/msg00369.html