From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH] x86_emulate: split the {reg,
	mem} union in struct operand.
Date: Thu, 26 Mar 2015 12:25:55 +0000
Message-ID: <5513FAD3.9050400@citrix.com>
References: <1427372174-980-1-git-send-email-tim@xen.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1427372174-980-1-git-send-email-tim@xen.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Tim Deegan <tim@xen.org>, xen-devel@lists.xen.org
Cc: Keir Fraser <keir@xen.org>, Jan Beulich <jbeulich@suse.com>
List-Id: xen-devel@lists.xenproject.org

On 26/03/15 12:16, Tim Deegan wrote:
> In the hopes of making any future errors along the lines of XSA-123
> into clean crashes instead of memory corruption bugs.
>
> Signed-off-by: Tim Deegan <tim@xen.org>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>

I am very much in favour of this change.

Does REG_POISON work with a 32bit compile of the test harness?

Independent of this, it might be easier to spot in crashes if the upper 
word of the address was also 8086

~Andrew

> ---
>   xen/arch/x86/x86_emulate/x86_emulate.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
> index c082c9e..37d1fea 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -319,7 +319,7 @@ struct operand {
>       /* Original operand value. */
>       unsigned long orig_val;
>   
> -    union {
> +    struct {
>           /* OP_REG: Pointer to register field. */
>           unsigned long *reg;
>           /* OP_MEM: Segment and offset. */
> @@ -329,6 +329,7 @@ struct operand {
>           } mem;
>       };
>   };
> +#define REG_POISON ((unsigned long *) 0x8000000000008086UL) /* non-canonical */
>   
>   typedef union {
>       uint64_t mmx;
> @@ -1447,14 +1448,15 @@ x86_emulate(
>       unsigned int op_bytes, def_op_bytes, ad_bytes, def_ad_bytes;
>       bool_t lock_prefix = 0;
>       int override_seg = -1, rc = X86EMUL_OKAY;
> -    struct operand src, dst;
> +    struct operand src = { .reg = REG_POISON };
> +    struct operand dst = { .reg = REG_POISON };
>       enum x86_swint_type swint_type;
>       DECLARE_ALIGNED(mmval_t, mmval);
>       /*
>        * Data operand effective address (usually computed from ModRM).
>        * Default is a memory operand relative to segment DS.
>        */
> -    struct operand ea = { .type = OP_MEM };
> +    struct operand ea = { .type = OP_MEM, .reg = REG_POISON };
>       ea.mem.seg = x86_seg_ds; /* gcc may reject anon union initializer */
>   
>       ctxt->retire.byte = 0;