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

Re: [Xen-devel] [PATCH] trace: further security fixes: add compiler memo

To: Jan Beulich <JBeulich@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] trace: further security fixes: add compiler memory barriers
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Mon, 5 Jul 2010 09:06:52 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 05 Jul 2010 01:07:54 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4C31AD8C02000078000097D0@xxxxxxxxxxxxxxxxxx>
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
Thread-index: AcscGH0/UbV4oP5eSOartTeQ1q6KYAAAIlQN
Thread-topic: [Xen-devel] [PATCH] trace: further security fixes: add compiler memory barriers
User-agent: Microsoft-Entourage/12.24.0.100205
I think I already applied this patch.

 K.

On 05/07/2010 09:01, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:

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



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>