[PATCH] xentrace: fix bug in t_info size

[PATCH] xentrace: fix bug in t_info size

[George Dunlap]

t_info size should be in bytes, not pages.  This fixes a bug
that crashes the hypervisor if the total number of all pages
is more than 1024 but less than 2048.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>

diff -r caea94988515 -r e633befe28ec xen/common/trace.c
--- a/xen/common/trace.c	Fri May 07 11:45:18 2010 +0100
+++ b/xen/common/trace.c	Fri May 07 19:20:52 2010 -0500
@@ -340,7 +340,7 @@
     case XEN_SYSCTL_TBUFOP_get_info:
         tbc->evt_mask   = tb_event_mask;
         tbc->buffer_mfn = t_info ? virt_to_mfn(t_info) : 0;
-        tbc->size = T_INFO_PAGES;
+        tbc->size = T_INFO_PAGES * PAGE_SIZE;
         break;
     case XEN_SYSCTL_TBUFOP_set_cpu_mask:
         xenctl_cpumap_to_cpumask(&tb_cpu_mask, &tbc->cpu_mask);

Re: [PATCH] xentrace: fix bug in t_info size

[Jeremy Fitzhardinge]

On 05/07/2010 05:25 PM, George Dunlap wrote:
> t_info size should be in bytes, not pages.  This fixes a bug
> that crashes the hypervisor if the total number of all pages
> is more than 1024 but less than 2048.
>   

Could this be causing other memory corruption too?

    J

> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>
> diff -r caea94988515 -r e633befe28ec xen/common/trace.c
> --- a/xen/common/trace.c	Fri May 07 11:45:18 2010 +0100
> +++ b/xen/common/trace.c	Fri May 07 19:20:52 2010 -0500
> @@ -340,7 +340,7 @@
>      case XEN_SYSCTL_TBUFOP_get_info:
>          tbc->evt_mask   = tb_event_mask;
>          tbc->buffer_mfn = t_info ? virt_to_mfn(t_info) : 0;
> -        tbc->size = T_INFO_PAGES;
> +        tbc->size = T_INFO_PAGES * PAGE_SIZE;
>          break;
>      case XEN_SYSCTL_TBUFOP_set_cpu_mask:
>          xenctl_cpumap_to_cpumask(&tb_cpu_mask, &tbc->cpu_mask);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>   

Re: [PATCH] xentrace: fix bug in t_info size

[George Dunlap]

I don't think so...  The entire xen structure actually is allocated, and 
the bounds checking makes sure nothing goes off the end of it.  It's 
just that (before this patch) xentrace only maps one of the two pages 
when it maps t_info.  It then happily passes who knows what into 
xc_map_foreign_range().

Arguably, passing junk into xc_map_foreign_range() shouldn't crash Xen; 
but that's a slightly different issue.

 -George

Jeremy Fitzhardinge wrote:
> On 05/07/2010 05:25 PM, George Dunlap wrote:
>   
>> t_info size should be in bytes, not pages.  This fixes a bug
>> that crashes the hypervisor if the total number of all pages
>> is more than 1024 but less than 2048.
>>   
>>     
>
> Could this be causing other memory corruption too?
>
>     J
>
>   
>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>>
>> diff -r caea94988515 -r e633befe28ec xen/common/trace.c
>> --- a/xen/common/trace.c	Fri May 07 11:45:18 2010 +0100
>> +++ b/xen/common/trace.c	Fri May 07 19:20:52 2010 -0500
>> @@ -340,7 +340,7 @@
>>      case XEN_SYSCTL_TBUFOP_get_info:
>>          tbc->evt_mask   = tb_event_mask;
>>          tbc->buffer_mfn = t_info ? virt_to_mfn(t_info) : 0;
>> -        tbc->size = T_INFO_PAGES;
>> +        tbc->size = T_INFO_PAGES * PAGE_SIZE;
>>          break;
>>      case XEN_SYSCTL_TBUFOP_set_cpu_mask:
>>          xenctl_cpumap_to_cpumask(&tb_cpu_mask, &tbc->cpu_mask);
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>>
>>   
>>     
>
>