[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v12 01/34] ARM: vGIC: avoid rank lock when reading priority
Hi,
On 06/19/2017 11:44 PM, Stefano Stabellini wrote:
On Mon, 19 Jun 2017, Julien Grall wrote:
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?
Yes, I think it makes sense. Do you release-ack it?
Release-acked-by: Julien Grall <julien.grall@xxxxxxx>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
