Re: [Xen-devel] [PATCH 5/7] xen/arm: traps: Avoid BUG_ON() in do_trap_brk()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 30 Jul 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 7/29/19 11:02 PM, Stefano Stabellini wrote:
> > On Tue, 23 Jul 2019, Julien Grall wrote:
> > > At the moment, do_trap_brk() is using a BUG_ON() to check the hardware
> > > has been correctly configured during boot.
> > > 
> > > Any error when configuring the hardware could result to a guest 'brk'
> > > trapping in the hypervisor and crash it.
> > > 
> > > This is pretty harsh to kill Xen when actually killing the guest would
> > > be enough as misconfiguring this trap would not lead to exposing
> > > sensitive data. Replace the BUG_ON() with crashing the guest.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > ---
> > >   xen/arch/arm/traps.c | 11 ++++++++---
> > >   1 file changed, 8 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > > index 132686ee0f..ef37ca6bde 100644
> > > --- a/xen/arch/arm/traps.c
> > > +++ b/xen/arch/arm/traps.c
> > > @@ -1304,10 +1304,15 @@ int do_bug_frame(const struct cpu_user_regs *regs,
> > > vaddr_t pc)
> > >   #ifdef CONFIG_ARM_64
> > >   static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
> > >   {
> > > -    /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive
> > > -     * software breakpoint exception for EL1 and EL0 here.
> > > +    /*
> > > +     * HCR_EL2.TGE and MDCR_EL2.TDR are currently not set. So we should
> > > +     * never receive software breakpoing exception for EL1 and EL0 here.
> > >        */
> > > -    BUG_ON(!hyp_mode(regs));
> > > +    if ( !hyp_mode(regs) )
> > > +    {
> > > +        domain_crash(current->domain);
> > > +        return;
> > > +    }
> > 
> > This is a good change to have. I am wondering if it would make sense to
> > also printk some debug messages, maybe even show_execution_state. If so,
> > we need to be careful that it's only done in debug builds, or it is rate
> > limited. What do you think?
> 
> Messages are already printed by domain_crash(...). But I don't see the concern
> about non-debug build or even not rate limiting... If the domain crash, then
> it will not cause anymore print and therefore you can't spam the console here.

Ah yes, you are quite right

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel