Re: [PATCH 2 of 3] xentrace: use tbuf_size for overflow check

[Olaf Hering <olaf@aepfle.de>]

On Fri, Apr 01, George Dunlap wrote:

> On Wed, 2011-03-30 at 19:04 +0100, Olaf Hering wrote:
> > # HG changeset patch
> > # User Olaf Hering <olaf@aepfle.de>
> > # Date 1301424075 -7200
> > # Node ID 3e95e737bc51c2295926e4f23389b1cb161d6d7b
> > # Parent  8a2ce5e49b2c5f2e013734b5d53eae37572f4101
> > xentrace: use tbuf_size for overflow check
> > 
> > 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.
> 
> Hmm -- while this is true, it's possible this may change in the future.
> If we ever changed t_info.tbuf_size to be u32, then t_buf.prod/cons
> would again be the limiting factor.
> 
> Should we perhaps add both checks?

I will test the change below.


diff -r 2b66b83b19b6 xen/common/trace.c
--- a/xen/common/trace.c	Thu Apr 07 12:13:58 2011 +0100
+++ b/xen/common/trace.c	Thu Apr 07 15:45:21 2011 +0200
@@ -104,25 +104,33 @@
  * 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);