Re: [PATCH] xentrace: correct overflow check for number of per-cpu trace pages

[George Dunlap <george.dunlap@citrix.com>]

Thanks Olaf.

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

On Thu, 2011-04-14 at 16:31 +0100, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1302794855 -7200
> # Node ID ceb2bbced07a03d938ff2f38ee6a5b88fa3f26ef
> # Parent  b5165fb66b56d9438d77b475eaa9db67318d1ea1
> xentrace: correct overflow check for number of per-cpu trace pages
> 
> The calculated number of per-cpu trace pages is stored in t_info and
> shared with tools like xentrace. Since its an u16 the value may overflow
> because the current check is based on u32.
> Using the u16 means each cpu could in theory use up to 256MB as trace
> buffer. However such a large allocation will currently fail on x86 due
> to the MAX_ORDER limit.
> Check both max theoretical number of pages per cpu and max number of
> pages reachable by struct t_buf->prod/cons variables with requested
> number of pages.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r b5165fb66b56 -r ceb2bbced07a xen/common/trace.c
> --- a/xen/common/trace.c	Thu Apr 14 14:57:24 2011 +0100
> +++ b/xen/common/trace.c	Thu Apr 14 17:27:35 2011 +0200
> @@ -104,25 +104,33 @@ static void calc_tinfo_first_offset(void
>   * calculate_tbuf_size - check to make sure that the proposed size will fit
>   * in the currently sized struct t_info and allows prod and cons to
>   * reach double the value without overflow.
> + * The t_info layout is fixed and cant be changed without breaking xentrace.
>   * Initialize t_info_pages based on number of trace pages.
>   */
>  static int calculate_tbuf_size(unsigned int pages)
>  {
> -    struct t_buf dummy;
> -    typeof(dummy.prod) size;
> +    struct t_buf dummy_size;
> +    typeof(dummy_size.prod) max_size;
> +    struct t_info dummy_pages;
> +    typeof(dummy_pages.tbuf_size) max_pages;
>      unsigned int t_info_words;
>  
>      /* force maximum value for an unsigned type */
> -    size = -1;
> +    max_size = -1;
> +    max_pages = -1;
>  
>      /* max size holds up to n pages */
> -    size /= PAGE_SIZE;
> -    if ( pages > size )
> +    max_size /= PAGE_SIZE;
> +
> +    if ( max_size < max_pages )
> +        max_pages = max_size;
> +
> +    if ( pages > max_pages )
>      {
>          printk(XENLOG_INFO "xentrace: requested number of %u pages "
>                 "reduced to %u\n",
> -               pages, (unsigned int)size);
> -        pages = size;
> +               pages, max_pages);
> +        pages = max_pages;
>      }
>  
>      t_info_words = num_online_cpus() * pages * sizeof(uint32_t);