[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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.