diff mbox

[1/3] xen/arm: Don't emulate the MMIO access if the instruction syndrome is invalid

Message ID 1374602713-716-2-git-send-email-julien.grall@linaro.org
State Superseded, archived
Headers show

Commit Message

Julien Grall July 23, 2013, 6:05 p.m. UTC
When the instruction syndrome is not valid, the transfer register is unknown.
If this register is used in the emulation code (it's the case for the VGIC),
Xen can retrieve wrong data.

For safety, consider invalid instruction syndrome as wrong memory access.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/traps.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Ian Campbell July 23, 2013, 10:10 p.m. UTC | #1
On Tue, 2013-07-23 at 19:05 +0100, Julien Grall wrote:
> When the instruction syndrome is not valid, the transfer register is unknown.

Are there known circumstances when this can happen? Trapped store
multiples or something like that? Did you actually see one?

> If this register is used in the emulation code (it's the case for the VGIC),
> Xen can retrieve wrong data.
> 
> For safety, consider invalid instruction syndrome as wrong memory access.

That's not really what it is though. I think this deserves at least a
printed warning but to be honest if we aren't going to emulate the
instruction then there isn't much chance that the guest will be able to
recover from a spurious dabt -- IOW we might as well just shoot the
guest in the head?

> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/traps.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index bbd60aa..d6dc37d 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1017,6 +1017,10 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>      if ( rc == -EFAULT )
>          goto bad_data_abort;
>  
> +    /* XXX: Decode the instruction if ISS is not valid */
> +    if ( !dabt.valid )
> +        goto bad_data_abort;
> +
>      if (handle_mmio(&info))
>      {
>          regs->pc += dabt.len ? 4 : 2;
diff mbox

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index bbd60aa..d6dc37d 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1017,6 +1017,10 @@  static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
     if ( rc == -EFAULT )
         goto bad_data_abort;
 
+    /* XXX: Decode the instruction if ISS is not valid */
+    if ( !dabt.valid )
+        goto bad_data_abort;
+
     if (handle_mmio(&info))
     {
         regs->pc += dabt.len ? 4 : 2;