Re: [Xen-devel] [PATCH] xen: add assertions in default_vcpu0_location to protect against broken masks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.06.12 at 11:47, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> When setting up the cpu sibling/etc masks on ARM I accidentally and
> incorrectly omitted a CPU from it's own sibling mask which caused this
> function to scribble over the heap. Add a couple of asserts to catch this in
> the future.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Keir (Xen.org) <keir@xxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> ---
>  xen/common/domctl.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 9f1a9ad..c1acd1d 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -190,10 +190,12 @@ static unsigned int default_vcpu0_location(cpumask_t 
> *online)
>       */
>      cpumask_copy(&cpu_exclude_map, per_cpu(cpu_sibling_mask, 0));
>      cpu = cpumask_first(&cpu_exclude_map);
> +    ASSERT(cpu < nr_cpus);
>      if ( cpumask_weight(&cpu_exclude_map) > 1 )
>          cpu = cpumask_next(cpu, &cpu_exclude_map);

You may want to add another ASSERT() here in case you care
about the difference between nr_cpus and nr_cpu_ids. Or move
the ASSERT() here if the difference is insignificant (as I would
think), as cpumask_next() invokes cpumask_check() (see also
below).

>      for_each_cpu(i, online)
>      {
> +        ASSERT(i < nr_cpus);

This one is almost redundant with the one in cpumask_check()
called from cpumask_test_cpu() (and the difference between
using nr_cpu_ids and nr_cpus doesn't seem relevant here), so
I'd recommend going with just the single earlier addition.

Jan

>          if ( cpumask_test_cpu(i, &cpu_exclude_map) )
>              continue;
>          if ( (i == cpumask_first(per_cpu(cpu_sibling_mask, i))) &&
> -- 
> 1.7.9.1




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel