xen/arm: traps: Don't panic when receiving an unknown debug trap

[PATCH] xen/arm: traps: Don't panic when receiving an unknown debug trap

Posted by Julien Grall 3 years, 5 months ago

From: Julien Grall <jgrall@amazon.com>

Even if debug trap are only meant for debugging purpose, it is quite
harsh to crash Xen if one of the trap sent by the guest is not handled.

So switch from a panic() to a printk().

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 8f40d0e0b6b1..a36f145e6739 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1410,7 +1410,7 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
         show_execution_state(regs);
         break;
     default:
-        panic("DOM%d: Unhandled debug trap %#x\n", domid, code);
+        printk("DOM%d: Unhandled debug trap %#x\n", domid, code);
         break;
     }
 }
-- 
2.17.1

Re: [PATCH] xen/arm: traps: Don't panic when receiving an unknown debug trap

Posted by Elliott Mitchell 3 years, 5 months ago

On Thu, Nov 05, 2020 at 10:31:06PM +0000, Julien Grall wrote:
> Even if debug trap are only meant for debugging purpose, it is quite
> harsh to crash Xen if one of the trap sent by the guest is not handled.
> 
> So switch from a panic() to a printk().

Might this qualify as security due to potential DoS?


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [PATCH] xen/arm: traps: Don't panic when receiving an unknown debug trap

Posted by Julien Grall 3 years, 5 months ago


On 05/11/2020 23:10, Elliott Mitchell wrote:
> On Thu, Nov 05, 2020 at 10:31:06PM +0000, Julien Grall wrote:
>> Even if debug trap are only meant for debugging purpose, it is quite
>> harsh to crash Xen if one of the trap sent by the guest is not handled.
>>
>> So switch from a panic() to a printk().
> 
> Might this qualify as security due to potential DoS?
This code path only exists with CONFIG_DEBUG=y which is not security 
supported.

Cheers,

-- 
Julien Grall

Re: [PATCH] xen/arm: traps: Don't panic when receiving an unknown debug trap

Posted by Bertrand Marquis 3 years, 5 months ago

Hi Julien,

> On 5 Nov 2020, at 22:31, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Even if debug trap are only meant for debugging purpose, it is quite
> harsh to crash Xen if one of the trap sent by the guest is not handled.
> 
> So switch from a panic() to a printk().

Very smart idea :-)

> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> xen/arch/arm/traps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 8f40d0e0b6b1..a36f145e6739 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1410,7 +1410,7 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
>         show_execution_state(regs);
>         break;
>     default:
> -        panic("DOM%d: Unhandled debug trap %#x\n", domid, code);
> +        printk("DOM%d: Unhandled debug trap %#x\n", domid, code);
>         break;
>     }
> }
> -- 
> 2.17.1
> 
>

Re: [PATCH] xen/arm: traps: Don't panic when receiving an unknown debug trap

Posted by Stefano Stabellini 3 years, 5 months ago

On Thu, 5 Nov 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Even if debug trap are only meant for debugging purpose, it is quite
> harsh to crash Xen if one of the trap sent by the guest is not handled.
> 
> So switch from a panic() to a printk().
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/traps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 8f40d0e0b6b1..a36f145e6739 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1410,7 +1410,7 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
>          show_execution_state(regs);
>          break;
>      default:
> -        panic("DOM%d: Unhandled debug trap %#x\n", domid, code);
> +        printk("DOM%d: Unhandled debug trap %#x\n", domid, code);
>          break;
>      }
>  }
> -- 
> 2.17.1
>