Re: [Xen-devel] [PATCH v12 01/34] ARM: vGIC: avoid rank lock when reading priority

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 14/06/17 18:55, Julien Grall wrote:
> Hi Andre,
>
> On 06/14/2017 05:51 PM, Andre Przywara wrote:
> > When reading the priority value of a virtual interrupt, we were taking
> > the respective rank lock so far.
> > However for forwarded interrupts (Dom0 only so far) this may lead to a
> > deadlock with the following call chain:
> > - MMIO access to change the IRQ affinity, calling the ITARGETSR handler
> > - this handler takes the appropriate rank lock and calls
> > vgic_store_itargetsr()
> > - vgic_store_itargetsr() will eventually call vgic_migrate_irq()
> > - if this IRQ is already in-flight, it will remove it from the old
> >    VCPU and inject it into the new one, by calling vgic_vcpu_inject_irq()
> > - vgic_vcpu_inject_irq will call vgic_get_virq_priority()
> > - vgic_get_virq_priority() tries to take the rank lock - again!
> > It seems like this code path has never been exercised before.
> >
> > Fix this by avoiding taking the lock in vgic_get_virq_priority() (like we
> > do in vgic_get_target_vcpu()).
> > Actually we are just reading one byte, and priority changes while
> > interrupts are handled are a benign race that can happen on real hardware
> > too. So it is safe to just prevent the compiler from reading from the
> > struct more than once.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> Reviewed-by: Julien Grall <julien.grall@xxxxxxx>
As you mentioned in the cover letter, I think it would be good to get 
this fixed in Xen 4.9. Stefano what do you think?
Cheers,

--
Julien Grall

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