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

[Julien Grall <julien.grall@xxxxxxx>]

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>

Cheers,

--
Julien Grall

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