[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> 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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.