Re: [PATCH 2/2] xen/common: llc-coloring: reject empty color tokens

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

+cc Carlo, Marco, Luca

On 16-May-26 17:03, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> 
> parse_color_config() currently accepts delimiters where a color value
> is expected because simple_strtoul() returns zero without advancing the
> input pointer. This makes strings such as ",2-6", "-10,19-20" or
> "1,,2" look as if an empty value was color 0.
> 
> Also add the missing newline to the DT color parsing error message.
> 
> Fixes: 6cdea3444eaf ("xen/arm: add Dom0 cache coloring support")
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
I don't see anything in the user facing docs that would make me think the
current behavior is intentional, therefore:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

I will still give a chance for the original authors to chime in if needed.

~Michal

> ---
>  xen/common/llc-coloring.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/llc-coloring.c b/xen/common/llc-coloring.c
> index 2606cb0977..5d00d4b40e 100644
> --- a/xen/common/llc-coloring.c
> +++ b/xen/common/llc-coloring.c
> @@ -64,14 +64,21 @@ static int __init parse_color_config(const char *buf, 
> unsigned int colors[],
>  
>      while ( *s != '\0' )
>      {
> +        const char *endp;
>          unsigned int color, start, end;
>  
> -        start = simple_strtoul(s, &s, 0);
> +        start = simple_strtoul(s, &endp, 0);
> +        if ( endp == s )
> +            goto fail;
> +        s = endp;
>  
>          if ( *s == '-' )    /* Range */
>          {
>              s++;
> -            end = simple_strtoul(s, &s, 0);
> +            end = simple_strtoul(s, &endp, 0);
> +            if ( endp == s )
> +                goto fail;
> +            s = endp;
>          }
>          else                /* Single value */
>              end = start;
> @@ -334,7 +341,7 @@ int __init domain_set_llc_colors_from_str(struct domain 
> *d, const char *str)
>      err = parse_color_config(str, colors, max_nr_colors, &num_colors);
>      if ( err )
>      {
> -        printk(XENLOG_ERR "Error parsing LLC color configuration");
> +        printk(XENLOG_ERR "Error parsing LLC color configuration\n");
>          xfree(colors);
>          return err;
>      }