WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] trace: further security fixes: add compiler memory b

To: "George Dunlap" <George.Dunlap@xxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] trace: further security fixes: add compiler memory barriers
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Mon, 05 Jul 2010 09:01:48 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 05 Jul 2010 01:03:08 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
This is to ensure fields shared writably with Dom0 get read only once
for any consistency checking followed by actual calculations.

I realized there was another multiple-read issue, a fix for which is
also included (which at once simplifies __insert_record()).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

--- a/xen/common/trace.c        2010-07-01 10:05:54.000000000 +0200
+++ b/xen/common/trace.c        2010-07-01 14:01:47.000000000 +0200
@@ -437,11 +437,13 @@ static inline bool_t bogus(u32 prod, u32
 static inline u32 calc_unconsumed_bytes(const struct t_buf *buf)
 {
     u32 prod = buf->prod, cons = buf->cons;
-    s32 x = prod - cons;
+    s32 x;
 
+    barrier(); /* must read buf->prod and buf->cons only once */
     if ( bogus(prod, cons) )
         return data_size;
 
+    x = prod - cons;
     if ( x < 0 )
         x += 2*data_size;
 
@@ -453,12 +455,14 @@ static inline u32 calc_unconsumed_bytes(
 
 static inline u32 calc_bytes_to_wrap(const struct t_buf *buf)
 {
-    u32 prod = buf->prod;
-    s32 x = data_size - prod;
+    u32 prod = buf->prod, cons = buf->cons;
+    s32 x;
 
-    if ( bogus(prod, buf->cons) )
+    barrier(); /* must read buf->prod and buf->cons only once */
+    if ( bogus(prod, cons) )
         return 0;
 
+    x = data_size - prod;
     if ( x <= 0 )
         x += data_size;
 
@@ -473,11 +477,14 @@ static inline u32 calc_bytes_avail(const
     return data_size - calc_unconsumed_bytes(buf);
 }
 
-static inline struct t_rec *next_record(const struct t_buf *buf)
+static inline struct t_rec *next_record(const struct t_buf *buf,
+                                        uint32_t *next)
 {
-    u32 x = buf->prod;
+    u32 x = buf->prod, cons = buf->cons;
 
-    if ( !tb_init_done || bogus(x, buf->cons) )
+    barrier(); /* must read buf->prod and buf->cons only once */
+    *next = x;
+    if ( !tb_init_done || bogus(x, cons) )
         return NULL;
 
     if ( x >= data_size )
@@ -504,23 +511,21 @@ static inline void __insert_record(volat
     BUG_ON(local_rec_size != rec_size);
     BUG_ON(extra & 3);
 
+    rec = next_record(buf, &next);
+    if ( !rec )
+        return;
     /* Double-check once more that we have enough space.
      * Don't bugcheck here, in case the userland tool is doing
      * something stupid. */
-    next = calc_bytes_avail(buf);
-    if ( next < rec_size )
+    if ( (unsigned char *)rec + rec_size > this_cpu(t_data) + data_size )
     {
         if ( printk_ratelimit() )
             printk(XENLOG_WARNING
-                   "%s: avail=%u (size=%08x prod=%08x cons=%08x) rec=%u\n",
-                   __func__, next, data_size, buf->prod, buf->cons, rec_size);
+                   "%s: size=%08x prod=%08x cons=%08x rec=%u\n",
+                   __func__, data_size, next, buf->cons, rec_size);
         return;
     }
-    rmb();
 
-    rec = next_record(buf);
-    if ( !rec )
-        return;
     rec->event = event;
     rec->extra_u32 = extra_word;
     dst = (unsigned char *)rec->u.nocycles.extra_u32;
@@ -537,9 +542,6 @@ static inline void __insert_record(volat
 
     wmb();
 
-    next = buf->prod;
-    if ( bogus(next, buf->cons) )
-        return;
     next += rec_size;
     if ( next >= 2*data_size )
         next -= 2*data_size;



Attachment: trace-barriers.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>