1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] spapr_pci: Fix interrupt leak in rtas_ibm_change_msi() error path

Greg Kurz posted 1 patch 5 years, 2 months ago
Test docker-mingw@fedora passed
Test asan passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/154956051753.218871.1505640778712413716.stgit@bahia.lab.toulouse-stg.fr.ibm.com
Maintainers: David Gibson <david@gibson.dropbear.id.au>
hw/ppc/spapr_pci.c |    6 ++++++
1 file changed, 6 insertions(+)
[Qemu-devel] [PATCH] spapr_pci: Fix interrupt leak in rtas_ibm_change_msi() error path
Posted by Greg Kurz 5 years, 2 months ago 
Now that IRQ allocation has been split in two (first allocate IRQ numbers,
then claim them), if the claiming fails, we must release the IRQs.

Fixes: 4fe75a8ccd80 "spapr: split the IRQ allocation sequence"
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 41d81f4a8500..6fe3c10c8d4c 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -393,6 +393,12 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     for (i = 0; i < req_num; i++) {
         spapr_irq_claim(spapr, irq + i, false, &err);
         if (err) {
+            if (i) {
+                spapr_irq_free(spapr, irq, i + 1);
+            }
+            if (!smc->legacy_irq_allocation) {
+                spapr_irq_msi_free(spapr, irq, req_num);
+            }
             error_reportf_err(err, "Can't allocate MSIs for device %x: ",
                               config_addr);
             rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
Re: [Qemu-devel] [PATCH] spapr_pci: Fix interrupt leak in rtas_ibm_change_msi() error path
Posted by David Gibson 5 years, 2 months ago 
On Thu, Feb 07, 2019 at 06:28:37PM +0100, Greg Kurz wrote:
> Now that IRQ allocation has been split in two (first allocate IRQ numbers,
> then claim them), if the claiming fails, we must release the IRQs.
> 
> Fixes: 4fe75a8ccd80 "spapr: split the IRQ allocation sequence"
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-4.0, thanks.

> ---
>  hw/ppc/spapr_pci.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 41d81f4a8500..6fe3c10c8d4c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -393,6 +393,12 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>      for (i = 0; i < req_num; i++) {
>          spapr_irq_claim(spapr, irq + i, false, &err);
>          if (err) {
> +            if (i) {
> +                spapr_irq_free(spapr, irq, i + 1);
> +            }
> +            if (!smc->legacy_irq_allocation) {
> +                spapr_irq_msi_free(spapr, irq, req_num);
> +            }
>              error_reportf_err(err, "Can't allocate MSIs for device %x: ",
>                                config_addr);
>              rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_pci: Fix interrupt leak in rtas_ibm_change_msi() error path
Posted by Greg Kurz 5 years, 2 months ago 
On Fri, 8 Feb 2019 10:06:47 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Feb 07, 2019 at 06:28:37PM +0100, Greg Kurz wrote:
> > Now that IRQ allocation has been split in two (first allocate IRQ numbers,
> > then claim them), if the claiming fails, we must release the IRQs.
> > 
> > Fixes: 4fe75a8ccd80 "spapr: split the IRQ allocation sequence"
> > Signed-off-by: Greg Kurz <groug@kaod.org>  
> 
> Applied to ppc-for-4.0, thanks.
> 

Oops I've just realized there's an off-by-one error...

> > ---
> >  hw/ppc/spapr_pci.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 41d81f4a8500..6fe3c10c8d4c 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -393,6 +393,12 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >      for (i = 0; i < req_num; i++) {
> >          spapr_irq_claim(spapr, irq + i, false, &err);
> >          if (err) {
> > +            if (i) {
> > +                spapr_irq_free(spapr, irq, i + 1);

... here. It should actually be:

+                spapr_irq_free(spapr, irq, i);

Can you fix this in your tree or should I post a v2 ?

> > +            }
> > +            if (!smc->legacy_irq_allocation) {
> > +                spapr_irq_msi_free(spapr, irq, req_num);
> > +            }
> >              error_reportf_err(err, "Can't allocate MSIs for device %x: ",
> >                                config_addr);
> >              rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> >   
>
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_pci: Fix interrupt leak in rtas_ibm_change_msi() error path
Posted by David Gibson 5 years, 2 months ago 
On Fri, Feb 08, 2019 at 12:15:03PM +0100, Greg Kurz wrote:
> On Fri, 8 Feb 2019 10:06:47 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Thu, Feb 07, 2019 at 06:28:37PM +0100, Greg Kurz wrote:
> > > Now that IRQ allocation has been split in two (first allocate IRQ numbers,
> > > then claim them), if the claiming fails, we must release the IRQs.
> > > 
> > > Fixes: 4fe75a8ccd80 "spapr: split the IRQ allocation sequence"
> > > Signed-off-by: Greg Kurz <groug@kaod.org>  
> > 
> > Applied to ppc-for-4.0, thanks.
> > 
> 
> Oops I've just realized there's an off-by-one error...
> 
> > > ---
> > >  hw/ppc/spapr_pci.c |    6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > > index 41d81f4a8500..6fe3c10c8d4c 100644
> > > --- a/hw/ppc/spapr_pci.c
> > > +++ b/hw/ppc/spapr_pci.c
> > > @@ -393,6 +393,12 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > >      for (i = 0; i < req_num; i++) {
> > >          spapr_irq_claim(spapr, irq + i, false, &err);
> > >          if (err) {
> > > +            if (i) {
> > > +                spapr_irq_free(spapr, irq, i + 1);
> 
> ... here. It should actually be:
> 
> +                spapr_irq_free(spapr, irq, i);
> 
> Can you fix this in your tree or should I post a v2 ?

Fixed up inline.

> 
> > > +            }
> > > +            if (!smc->legacy_irq_allocation) {
> > > +                spapr_irq_msi_free(spapr, irq, req_num);
> > > +            }
> > >              error_reportf_err(err, "Can't allocate MSIs for device %x: ",
> > >                                config_addr);
> > >              rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> > >   
> > 
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_pci: Fix interrupt leak in rtas_ibm_change_msi() error path
Posted by Greg Kurz 5 years, 2 months ago 
On Fri, 8 Feb 2019 22:33:36 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Feb 08, 2019 at 12:15:03PM +0100, Greg Kurz wrote:
> > On Fri, 8 Feb 2019 10:06:47 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Thu, Feb 07, 2019 at 06:28:37PM +0100, Greg Kurz wrote:  
> > > > Now that IRQ allocation has been split in two (first allocate IRQ numbers,
> > > > then claim them), if the claiming fails, we must release the IRQs.
> > > > 
> > > > Fixes: 4fe75a8ccd80 "spapr: split the IRQ allocation sequence"
> > > > Signed-off-by: Greg Kurz <groug@kaod.org>    
> > > 
> > > Applied to ppc-for-4.0, thanks.
> > >   
> > 
> > Oops I've just realized there's an off-by-one error...
> >   
> > > > ---
> > > >  hw/ppc/spapr_pci.c |    6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > > > index 41d81f4a8500..6fe3c10c8d4c 100644
> > > > --- a/hw/ppc/spapr_pci.c
> > > > +++ b/hw/ppc/spapr_pci.c
> > > > @@ -393,6 +393,12 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > > >      for (i = 0; i < req_num; i++) {
> > > >          spapr_irq_claim(spapr, irq + i, false, &err);
> > > >          if (err) {
> > > > +            if (i) {
> > > > +                spapr_irq_free(spapr, irq, i + 1);  
> > 
> > ... here. It should actually be:
> > 
> > +                spapr_irq_free(spapr, irq, i);
> > 
> > Can you fix this in your tree or should I post a v2 ?  
> 
> Fixed up inline.
> 

Thanks !

> >   
> > > > +            }
> > > > +            if (!smc->legacy_irq_allocation) {
> > > > +                spapr_irq_msi_free(spapr, irq, req_num);
> > > > +            }
> > > >              error_reportf_err(err, "Can't allocate MSIs for device %x: ",
> > > >                                config_addr);
> > > >              rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> > > >     
> > >   
> >   
> 
> 
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.