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] Re: [PATCH 5/6] trace: fix security issues

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH 5/6] trace: fix security issues
From: George Dunlap <dunlapg@xxxxxxxxx>
Date: Wed, 30 Jun 2010 17:58:29 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 30 Jun 2010 10:00:11 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=nypbNE3+XapEaKMgHsk2nWnDZ+5bzY9kNuAjKjGiAtw=; b=bydKKOJgwGMugrJA0U4r9Bv3gact4nDGadCTQw6ke75cOaV/TicRHdxRnUTz5/o3MZ VYs4rgyDn2OEmD1mZUESv5xLGl37j1JIgiRjWfxoWlx6Iz/AeUdCVd2XUKR8O+EMedZh pVvgVtcakxWP0iXmS2OHb+tMCXWHTnOG/TLjI=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=TEGP69Gy7MlAxiSUky/RmoBmgRLBZ2f+WHfKTyd6sfeVOwRtUzSkz+DGMuHP4ymYOm rnxhSPaFKGIwrTWJZlLwf7ncMCOboOJgLQyvl39GvE6PnS36tClvczkysZwgH7NcTqPT h7x7T08dKr7XqkLKVM/Ze06LFbDcyL5JGLeXc=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4C2A2F710200007800008A90@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>
References: <4C2A2F710200007800008A90@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Most of the patch, other than the volatiles, looks good; thanks for doing this.

Can you explain exactly what it is you're trying to accomplish by
making things volatile?  What kinds of failure modes are you expecting
to protect against?  Glancing through the code, I can't really see
that making things volatile will make much of a difference.

Further comments inline below.

On Tue, Jun 29, 2010 at 4:37 PM, Jan Beulich <JBeulich@xxxxxxxxxx> wrote:
> --- 2010-06-15.orig/xen/common/trace.c  2010-06-29 17:04:45.000000000 +0200
> +++ 2010-06-15/xen/common/trace.c       2010-06-29 17:05:09.000000000 +0200
[snip]
> @@ -626,13 +649,11 @@ void __trace_var(u32 event, int cycles,
>
>     /* Read tb_init_done /before/ t_bufs. */
>     rmb();
> -
> -    spin_lock_irqsave(&this_cpu(t_lock), flags);
> -
>     buf = this_cpu(t_bufs);
> -
>     if ( unlikely(!buf) )
> -        goto unlock;
> +        return;
> +
> +    spin_lock_irqsave(&this_cpu(t_lock), flags);

This isn't any good: the whole point of this lock is to keep t_bufs
from disappearing under our feet.

>
>     started_below_highwater = (calc_unconsumed_bytes(buf) < t_buf_highwater);
>
> --- 2010-06-15.orig/xen/include/xen/trace.h     2010-06-29 16:53:25.000000000 
> +0200
> +++ 2010-06-15/xen/include/xen/trace.h  2010-06-25 12:08:36.000000000 +0200
> @@ -36,7 +36,7 @@ int tb_control(struct xen_sysctl_tbuf_op
>
>  int trace_will_trace_event(u32 event);
>
> -void __trace_var(u32 event, int cycles, int extra, unsigned char 
> *extra_data);
> +void __trace_var(u32 event, bool_t cycles, unsigned int extra, const void *);
>
>  static inline void trace_var(u32 event, int cycles, int extra,
>                                unsigned char *extra_data)
> @@ -57,7 +57,7 @@ static inline void trace_var(u32 event,
>         {                                                       \
>             u32 _d[1];                                          \
>             _d[0] = d1;                                         \
> -            __trace_var(_e, 1, sizeof(*_d), (unsigned char *)_d); \
> +            __trace_var(_e, 1, sizeof(_d), _d);                 \
>         }                                                       \
>     } while ( 0 )
>
> @@ -68,7 +68,7 @@ static inline void trace_var(u32 event,
>             u32 _d[2];                                          \
>             _d[0] = d1;                                         \
>             _d[1] = d2;                                         \
> -            __trace_var(_e, 1, sizeof(*_d)*2, (unsigned char *)_d); \
> +            __trace_var(_e, 1, sizeof(_d), _d);                 \
>         }                                                       \
>     } while ( 0 )
>
> @@ -80,7 +80,7 @@ static inline void trace_var(u32 event,
>             _d[0] = d1;                                         \
>             _d[1] = d2;                                         \
>             _d[2] = d3;                                         \
> -            __trace_var(_e, 1, sizeof(*_d)*3, (unsigned char *)_d); \
> +            __trace_var(_e, 1, sizeof(_d), _d);                 \
>         }                                                       \
>     } while ( 0 )
>
> @@ -93,7 +93,7 @@ static inline void trace_var(u32 event,
>             _d[1] = d2;                                         \
>             _d[2] = d3;                                         \
>             _d[3] = d4;                                         \
> -            __trace_var(_e, 1, sizeof(*_d)*4, (unsigned char *)_d); \
> +            __trace_var(_e, 1, sizeof(_d), _d);                 \
>         }                                                       \
>     } while ( 0 )
>
> @@ -107,7 +107,7 @@ static inline void trace_var(u32 event,
>             _d[2] = d3;                                         \
>             _d[3] = d4;                                         \
>             _d[4] = d5;                                         \
> -            __trace_var(_e, 1, sizeof(*_d)*5, (unsigned char *)_d); \
> +            __trace_var(_e, 1, sizeof(_d), _d);                 \
>         }                                                       \
>     } while ( 0 )
>
> @@ -122,7 +122,7 @@ static inline void trace_var(u32 event,
>             _d[3] = d4;                                         \
>             _d[4] = d5;                                         \
>             _d[5] = d6;                                         \
> -            __trace_var(_e, 1, sizeof(*_d)*6, (unsigned char *)_d); \
> +            __trace_var(_e, 1, sizeof(_d), _d);                 \
>         }                                                       \
>     } while ( 0 )
>
>
>
>

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

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