Re: [PATCH v3 4/8] x86/hvm: convert gsi_assert_count into a variable size array

[Roger Pau Monne <roger.pau@citrix.com>]

On Fri, Mar 31, 2017 at 09:16:53AM -0600, Jan Beulich wrote:
> >>> On 29.03.17 at 16:47, <roger.pau@citrix.com> wrote:
> > Rearrange the fields of hvm_irq so that gsi_assert_count can be converted into
> > a variable size array and add a new field to account the number of GSIs.
> > 
> > Due to this changes the irq member in the hvm_domain struct also needs to
> > become a pointer set at runtime.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with one possible adjustment:
> 
> > --- a/xen/arch/x86/hvm/irq.c
> > +++ b/xen/arch/x86/hvm/irq.c
> > @@ -69,6 +69,7 @@ static void __hvm_pci_intx_assert(
> >          return;
> >  
> >      gsi = hvm_pci_intx_gsi(device, intx);
> > +    ASSERT(gsi < hvm_irq->nr_gsis);
> >      if ( hvm_irq->gsi_assert_count[gsi]++ == 0 )
> >          assert_gsi(d, gsi);
> >  
> > @@ -99,6 +100,7 @@ static void __hvm_pci_intx_deassert(
> >          return;
> >  
> >      gsi = hvm_pci_intx_gsi(device, intx);
> > +    ASSERT(gsi < hvm_irq->nr_gsis);
> >      --hvm_irq->gsi_assert_count[gsi];
> 
> These ASSERT()s certainly don't make the situation worse after
> just this patch alone, but once the higher Dom0 count comes
> into play them turning into nothing on release builds is sub-
> optimal. In a few cases we've used (or at least have been
> considering using) something like
> 
>     if ( gsi >= hvm_irq->nr_gsis)
>     {
>         ASSERT_UNREACHABLE();
>         return;
>     }
> 
> to avoid the array overrun even in the release build case.
> Otoh it's only Dom0 which could actually cause this, so it
> wouldn't be a security issue as per our current classification.
> Andrew - do you have a specific opinion either way here?

Yes, I think your suggestion is better, it will trigger on debug builds while
not causing array overrun on production ones.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel