1. Patchew
  2. Xen
  3. View series

[PATCH] xen/irq: Propagate the error from init_one_desc_irq() in init_irq_data()

Julien Grall posted 1 patch 3 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20201119145434.28065-1-julien@xen.org
Maintainers: "Roger Pau Monné" <roger.pau@citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Stefano Stabellini <sstabellini@kernel.org>, Julien Grall <julien@xen.org>, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>, Wei Liu <wl@xen.org>, Jan Beulich <jbeulich@suse.com>
xen/arch/arm/irq.c | 7 ++++++-
xen/arch/x86/irq.c | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
[PATCH] xen/irq: Propagate the error from init_one_desc_irq() in init_irq_data()
Posted by Julien Grall 3 years, 5 months ago 
From: Julien Grall <jgrall@amazon.com>

init_one_desc_irq() can return an error if it is unable to allocate
memory. While this is unlikely to happen during boot (called from
init_irq_data()), it is better to harden the code by propagting the
return value.

Spotted by coverity.

CID: 106529

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/irq.c | 7 ++++++-
 xen/arch/x86/irq.c | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 3877657a5277..279d221a2b85 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -88,7 +88,12 @@ static int __init init_irq_data(void)
     for ( irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++ )
     {
         struct irq_desc *desc = irq_to_desc(irq);
-        init_one_irq_desc(desc);
+        int rc;
+
+        rc = init_one_irq_desc(desc);
+        if ( rc )
+            return rc;
+
         desc->irq = irq;
         desc->action  = NULL;
     }
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 45966947919e..3ebd684415ac 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -428,9 +428,14 @@ int __init init_irq_data(void)
 
     for ( irq = 0; irq < nr_irqs_gsi; irq++ )
     {
+        int rc;
+
         desc = irq_to_desc(irq);
         desc->irq = irq;
-        init_one_irq_desc(desc);
+
+        rc = init_one_irq_desc(desc);
+        if ( rc )
+            return rc;
     }
     for ( ; irq < nr_irqs; irq++ )
         irq_to_desc(irq)->irq = irq;
-- 
2.17.1
Re: [PATCH] xen/irq: Propagate the error from init_one_desc_irq() in init_irq_data()
Posted by Bertrand Marquis 3 years, 5 months ago 
Hi,

> On 19 Nov 2020, at 14:54, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> init_one_desc_irq() can return an error if it is unable to allocate
> memory. While this is unlikely to happen during boot (called from
> init_irq_data()), it is better to harden the code by propagting the
> return value.
> 
> Spotted by coverity.
> 
> CID: 106529
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand


> ---
> xen/arch/arm/irq.c | 7 ++++++-
> xen/arch/x86/irq.c | 7 ++++++-
> 2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 3877657a5277..279d221a2b85 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -88,7 +88,12 @@ static int __init init_irq_data(void)
>     for ( irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++ )
>     {
>         struct irq_desc *desc = irq_to_desc(irq);
> -        init_one_irq_desc(desc);
> +        int rc;
> +
> +        rc = init_one_irq_desc(desc);
> +        if ( rc )
> +            return rc;
> +
>         desc->irq = irq;
>         desc->action  = NULL;
>     }
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index 45966947919e..3ebd684415ac 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -428,9 +428,14 @@ int __init init_irq_data(void)
> 
>     for ( irq = 0; irq < nr_irqs_gsi; irq++ )
>     {
> +        int rc;
> +
>         desc = irq_to_desc(irq);
>         desc->irq = irq;
> -        init_one_irq_desc(desc);
> +
> +        rc = init_one_irq_desc(desc);
> +        if ( rc )
> +            return rc;
>     }
>     for ( ; irq < nr_irqs; irq++ )
>         irq_to_desc(irq)->irq = irq;
> -- 
> 2.17.1
> 
>
Re: [PATCH] xen/irq: Propagate the error from init_one_desc_irq() in init_irq_data()
Posted by Roger Pau Monné 3 years, 5 months ago 
On Thu, Nov 19, 2020 at 02:54:34PM +0000, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> init_one_desc_irq() can return an error if it is unable to allocate
> memory. While this is unlikely to happen during boot (called from
> init_irq_data()), it is better to harden the code by propagting the
> return value.
> 
> Spotted by coverity.
> 
> CID: 106529
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

> ---
>  xen/arch/x86/irq.c | 7 ++++++-

Fox x86:
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 3877657a5277..279d221a2b85 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -88,7 +88,12 @@ static int __init init_irq_data(void)
>      for ( irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++ )
>      {
>          struct irq_desc *desc = irq_to_desc(irq);
> -        init_one_irq_desc(desc);
> +        int rc;
> +
> +        rc = init_one_irq_desc(desc);

You could init rc at definition.

Thanks, Roger.
Re: [PATCH] xen/irq: Propagate the error from init_one_desc_irq() in init_irq_data()
Posted by Julien Grall 3 years, 4 months ago 
Hi Roger,

On 19/11/2020 15:11, Roger Pau Monné wrote:
> On Thu, Nov 19, 2020 at 02:54:34PM +0000, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> init_one_desc_irq() can return an error if it is unable to allocate
>> memory. While this is unlikely to happen during boot (called from
>> init_irq_data()), it is better to harden the code by propagting the
>> return value.
>>
>> Spotted by coverity.
>>
>> CID: 106529
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
>> ---
>>   xen/arch/x86/irq.c | 7 ++++++-
> 
> Fox x86:
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thank you!

> 
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
>> index 3877657a5277..279d221a2b85 100644
>> --- a/xen/arch/arm/irq.c
>> +++ b/xen/arch/arm/irq.c
>> @@ -88,7 +88,12 @@ static int __init init_irq_data(void)
>>       for ( irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++ )
>>       {
>>           struct irq_desc *desc = irq_to_desc(irq);
>> -        init_one_irq_desc(desc);
>> +        int rc;
>> +
>> +        rc = init_one_irq_desc(desc);
> 
> You could init rc at definition.

I need to send a new version, so I have merged the two line together.

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/irq: Propagate the error from init_one_desc_irq() in init_irq_data()
Posted by Stefano Stabellini 3 years, 5 months ago 
On Thu, 19 Nov 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> init_one_desc_irq() can return an error if it is unable to allocate
> memory. While this is unlikely to happen during boot (called from
> init_irq_data()), it is better to harden the code by propagting the
> return value.
> 
> Spotted by coverity.
> 
> CID: 106529
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Hi Julien,

Thanks for the patch. I was about to commit it when I realized there is
one more caller: xen/arch/arm/irq.c:init_local_irq_data

Should we change that too to check for the return error?


> ---
>  xen/arch/arm/irq.c | 7 ++++++-
>  xen/arch/x86/irq.c | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 3877657a5277..279d221a2b85 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -88,7 +88,12 @@ static int __init init_irq_data(void)
>      for ( irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++ )
>      {
>          struct irq_desc *desc = irq_to_desc(irq);
> -        init_one_irq_desc(desc);
> +        int rc;
> +
> +        rc = init_one_irq_desc(desc);
> +        if ( rc )
> +            return rc;
> +
>          desc->irq = irq;
>          desc->action  = NULL;
>      }
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index 45966947919e..3ebd684415ac 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -428,9 +428,14 @@ int __init init_irq_data(void)
>  
>      for ( irq = 0; irq < nr_irqs_gsi; irq++ )
>      {
> +        int rc;
> +
>          desc = irq_to_desc(irq);
>          desc->irq = irq;
> -        init_one_irq_desc(desc);
> +
> +        rc = init_one_irq_desc(desc);
> +        if ( rc )
> +            return rc;
>      }
>      for ( ; irq < nr_irqs; irq++ )
>          irq_to_desc(irq)->irq = irq;
> -- 
> 2.17.1
>
Re: [PATCH] xen/irq: Propagate the error from init_one_desc_irq() in init_irq_data()
Posted by Julien Grall 3 years, 4 months ago 

On 19/11/2020 23:44, Stefano Stabellini wrote:
> On Thu, 19 Nov 2020, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> init_one_desc_irq() can return an error if it is unable to allocate
>> memory. While this is unlikely to happen during boot (called from
>> init_irq_data()), it is better to harden the code by propagting the
>> return value.
>>
>> Spotted by coverity.
>>
>> CID: 106529
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Hi Julien,

Hi Stefano,

> Thanks for the patch. I was about to commit it when I realized there is
> one more caller: xen/arch/arm/irq.c:init_local_irq_data
> 
> Should we change that too to check for the return error?

We should change too. I will send a new version.

Cheers,

-- 
Julien Grall

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.