Hi Jan,
On Tue, Mar 19, 2024 at 4:37 PM Jan Beulich <jbeul...@suse.com> wrote:
>
> On 15.03.2024 11:58, Carlo Nonato wrote:
> > @@ -219,6 +220,39 @@ void domain_llc_coloring_free(struct domain *d)
> > xfree(__va(__pa(d->llc_colors)));
> > }
> >
> > +int domain_set_llc_colors(struct domain *d,
> > + const struct xen_domctl_set_llc_colors *config)
> > +{
> > + unsigned int *colors;
> > +
> > + if ( d->num_llc_colors )
> > + return -EEXIST;
> > +
> > + if ( !config->num_llc_colors )
> > + return domain_set_default_colors(d);
> > +
> > + if ( config->num_llc_colors > max_nr_colors || config->pad )
>
> The check of "pad" wants carrying out in all cases; I expect it wants
> moving to the caller.
Ok.
> > + return -EINVAL;
> > +
> > + colors = xmalloc_array(unsigned int, config->num_llc_colors);
> > + if ( !colors )
> > + return -ENOMEM;
> > +
> > + if ( copy_from_guest(colors, config->llc_colors,
> > config->num_llc_colors) )
> > + return -EFAULT;
>
> You're leaking "colors" when taking this or ...
>
> > + if ( !check_colors(colors, config->num_llc_colors) )
> > + {
> > + printk(XENLOG_ERR "Bad LLC color config for %pd\n", d);
> > + return -EINVAL;
>
> ... this error path.
You're right.
> Jan
Thanks.
On Tue, Mar 19, 2024 at 4:37 PM Jan Beulich <jbeul...@suse.com> wrote:
>
> On 15.03.2024 11:58, Carlo Nonato wrote:
> > @@ -219,6 +220,39 @@ void domain_llc_coloring_free(struct domain *d)
> > xfree(__va(__pa(d->llc_colors)));
> > }
> >
> > +int domain_set_llc_colors(struct domain *d,
> > + const struct xen_domctl_set_llc_colors *config)
> > +{
> > + unsigned int *colors;
> > +
> > + if ( d->num_llc_colors )
> > + return -EEXIST;
> > +
> > + if ( !config->num_llc_colors )
> > + return domain_set_default_colors(d);
> > +
> > + if ( config->num_llc_colors > max_nr_colors || config->pad )
>
> The check of "pad" wants carrying out in all cases; I expect it wants
> moving to the caller.
>
> > + return -EINVAL;
> > +
> > + colors = xmalloc_array(unsigned int, config->num_llc_colors);
> > + if ( !colors )
> > + return -ENOMEM;
> > +
> > + if ( copy_from_guest(colors, config->llc_colors,
> > config->num_llc_colors) )
> > + return -EFAULT;
>
> You're leaking "colors" when taking this or ...
>
> > + if ( !check_colors(colors, config->num_llc_colors) )
> > + {
> > + printk(XENLOG_ERR "Bad LLC color config for %pd\n", d);
> > + return -EINVAL;
>
> ... this error path.
>
> Jan
