Re: [Xen-devel] [PATCH] xen: Fix xenctl_cpumap_to_cpumask buffer size check

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.11.12 at 07:04, Matthew Daley <mattjd@xxxxxxxxx> wrote:
> xenctl_cpumap_to_cpumask incorrectly uses sizeof when checking whether
> bits should be masked off from the input cpumap bitmap or not.
> 
> Fix by using the correct cpumask buffer size in place of sizeof.
> 
> Signed-off-by: Matthew Daley <mattjd@xxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

However, I would have preferred the adjustment to be ...

> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index e153cb4..204e951 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -78,7 +78,7 @@ int xenctl_cpumap_to_cpumask(
>      {
>          if ( copy_from_guest(bytemap, xenctl_cpumap->bitmap, copy_bytes) )
>              err = -EFAULT;
> -        if ( (xenctl_cpumap->nr_cpus & 7) && (guest_bytes <= 
> sizeof(bytemap)) )
> +        if ( (xenctl_cpumap->nr_cpus & 7) && (guest_bytes <= (nr_cpu_ids + 
> 7) / 8) )

        if ( (xenctl_cpumap->nr_cpus & 7) && (guest_bytes <= copy_bytes) )

or even (considering that guest_bytes >= copy_bytes due to the
way copy_bytes gets initialized)

        if ( (xenctl_cpumap->nr_cpus & 7) && (guest_bytes == copy_bytes) )

to make explicit when exactly the masking is necessary.

Further, the fact this is not security relevant (i.e. the bug could
not cause memory corruption) isn't obvious either: It is implied
from _xmalloc() never returning chunks of data smaller than the
size of a pointer, i.e. even if sizeof(void*) > guest_bytes >
copy_bytes, the piece of memory erroneously written to would
still be inside the allocation done at the top of the function. I'd
suppose that would have been worth mentioning in the change
description.

And, for the record (and in order to determine backporting
needs), I caused this with c/s 23991:a7ccbc79fc17.

Jan

>              bytemap[guest_bytes-1] &= ~(0xff << (xenctl_cpumap->nr_cpus & 
> 7));
>      }
>  
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 




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