| Message ID | 1470348549-10855-2-git-send-email-keith.busch@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
This one's fine Acked-by Jon Derrick: <jonathan.derrick@intel.com> On Thu, Aug 04, 2016 at 04:09:09PM -0600, Keith Busch wrote: > This patch fixes a potential race when disabling MSI/MSI-x on a VMD domain > device. If the vmd interrupt service is running, it may see a disabled > irq. We can synchronize rcu just before freeing the msi descriptor. This > is safe since the irq_desc lock isn't held, and the descriptor is valid > even though it is disabled. After vmd_msi_free, though, the handler is > reinitialiazed to handle_bad_irq, so we can't let the vmd isr's list > iteration see the disabled irq after this. > > Signed-off-by: Keith Busch <keith.busch@intel.com> > --- > arch/x86/pci/vmd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c > index 2294907..e3c9b9e 100644 > --- a/arch/x86/pci/vmd.c > +++ b/arch/x86/pci/vmd.c > @@ -213,6 +213,8 @@ static void vmd_msi_free(struct irq_domain *domain, > struct vmd_irq *vmdirq = irq_get_chip_data(virq); > unsigned long flags; > > + synchronize_rcu(); > + > /* XXX: Potential optimization to rebalance */ > raw_spin_lock_irqsave(&list_lock, flags); > vmdirq->irq->count--; > -- > 2.7.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, Any chance of pulling this one in? Thanks, Jon On Mon, Aug 08, 2016 at 10:47:21AM -0600, Jon Derrick wrote: > This one's fine > > Acked-by Jon Derrick: <jonathan.derrick@intel.com> > > > On Thu, Aug 04, 2016 at 04:09:09PM -0600, Keith Busch wrote: > > This patch fixes a potential race when disabling MSI/MSI-x on a VMD domain > > device. If the vmd interrupt service is running, it may see a disabled > > irq. We can synchronize rcu just before freeing the msi descriptor. This > > is safe since the irq_desc lock isn't held, and the descriptor is valid > > even though it is disabled. After vmd_msi_free, though, the handler is > > reinitialiazed to handle_bad_irq, so we can't let the vmd isr's list > > iteration see the disabled irq after this. > > > > Signed-off-by: Keith Busch <keith.busch@intel.com> > > --- > > arch/x86/pci/vmd.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c > > index 2294907..e3c9b9e 100644 > > --- a/arch/x86/pci/vmd.c > > +++ b/arch/x86/pci/vmd.c > > @@ -213,6 +213,8 @@ static void vmd_msi_free(struct irq_domain *domain, > > struct vmd_irq *vmdirq = irq_get_chip_data(virq); > > unsigned long flags; > > > > + synchronize_rcu(); > > + > > /* XXX: Potential optimization to rebalance */ > > raw_spin_lock_irqsave(&list_lock, flags); > > vmdirq->irq->count--; > > -- > > 2.7.2 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 04, 2016 at 04:09:09PM -0600, Keith Busch wrote: > This patch fixes a potential race when disabling MSI/MSI-x on a VMD domain > device. If the vmd interrupt service is running, it may see a disabled > irq. We can synchronize rcu just before freeing the msi descriptor. This > is safe since the irq_desc lock isn't held, and the descriptor is valid > even though it is disabled. After vmd_msi_free, though, the handler is > reinitialiazed to handle_bad_irq, so we can't let the vmd isr's list > iteration see the disabled irq after this. > > Signed-off-by: Keith Busch <keith.busch@intel.com> Applied with Jon's ack to pci/host-vmd for v4.9, thanks! If anybody else reads this, this patch got lost for a while because it was posted as the second patch of a series. I had some comments on the first patch that led to a little rework. When that happens, I mark the *entire* series as "Changes requested" or "Superseded" because I'm expecting a complete repost with the rework. It's too much work for me to track individual patches within a series. So if you update one patch in a series, please just repost the entire series, including other patches that may be unmodified. Bjorn > --- > arch/x86/pci/vmd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c > index 2294907..e3c9b9e 100644 > --- a/arch/x86/pci/vmd.c > +++ b/arch/x86/pci/vmd.c > @@ -213,6 +213,8 @@ static void vmd_msi_free(struct irq_domain *domain, > struct vmd_irq *vmdirq = irq_get_chip_data(virq); > unsigned long flags; > > + synchronize_rcu(); > + > /* XXX: Potential optimization to rebalance */ > raw_spin_lock_irqsave(&list_lock, flags); > vmdirq->irq->count--; > -- > 2.7.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/pci/vmd.c b/arch/x86/pci/vmd.c index 2294907..e3c9b9e 100644 --- a/arch/x86/pci/vmd.c +++ b/arch/x86/pci/vmd.c @@ -213,6 +213,8 @@ static void vmd_msi_free(struct irq_domain *domain, struct vmd_irq *vmdirq = irq_get_chip_data(virq); unsigned long flags; + synchronize_rcu(); + /* XXX: Potential optimization to rebalance */ raw_spin_lock_irqsave(&list_lock, flags); vmdirq->irq->count--;
This patch fixes a potential race when disabling MSI/MSI-x on a VMD domain device. If the vmd interrupt service is running, it may see a disabled irq. We can synchronize rcu just before freeing the msi descriptor. This is safe since the irq_desc lock isn't held, and the descriptor is valid even though it is disabled. After vmd_msi_free, though, the handler is reinitialiazed to handle_bad_irq, so we can't let the vmd isr's list iteration see the disabled irq after this. Signed-off-by: Keith Busch <keith.busch@intel.com> --- arch/x86/pci/vmd.c | 2 ++ 1 file changed, 2 insertions(+)