diff mbox

[2/2] vmd: Synchronize with RCU freeing msi irq descs

Message ID 1470348549-10855-2-git-send-email-keith.busch@intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Keith Busch Aug. 4, 2016, 10:09 p.m. UTC
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(+)

Comments

Jon Derrick Aug. 8, 2016, 4:47 p.m. UTC | #1
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
Jon Derrick Aug. 29, 2016, 4:07 p.m. UTC | #2
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
Bjorn Helgaas Sept. 14, 2016, 10:10 p.m. UTC | #3
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 mbox

Patch

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--;