diff mbox

[1/2] xen/pt: Unlock d->event_lock on error paths

Message ID 1498585673-23268-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper June 27, 2017, 5:47 p.m. UTC 
Introduced by c/s fba00494268 "x86/pt: enable binding of GSIs to a PVH Dom0"

Spotted by Coverity.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/drivers/passthrough/io.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Jan Beulich June 27, 2017, 6:39 p.m. UTC | #1 
>>> Andrew Cooper <andrew.cooper3@citrix.com> 06/27/17 7:48 PM >>>
>Introduced by c/s fba00494268 "x86/pt: enable binding of GSIs to a PVH Dom0"
>
>Spotted by Coverity.
>
>Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

And I'm sorry for not having noticed while reviewing the original patch.

Jan
Roger Pau Monné June 28, 2017, 7:41 a.m. UTC | #2 
On Tue, Jun 27, 2017 at 06:47:52PM +0100, Andrew Cooper wrote:
> Introduced by c/s fba00494268 "x86/pt: enable binding of GSIs to a PVH Dom0"
> 
> Spotted by Coverity.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks!

Reivwed-by: Roger Pau Monné <roger.pau@citrix.com>

None of those paths should be used in any case.

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  xen/drivers/passthrough/io.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
> index 2fdbba6..25e3fb4 100644
> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -490,7 +490,11 @@ int pt_irq_create_bind(
>              /* MSI_TRANSLATE is not supported for the hardware domain. */
>              if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_PCI ||
>                   pirq >= hvm_domain_irq(d)->nr_gsis )
> +            {
> +                spin_unlock(&d->event_lock);
> +
>
>                  return -EINVAL;
> +            }
>              guest_gsi = pirq;
>          }
>  
> @@ -523,6 +527,8 @@ int pt_irq_create_bind(
>  
>                      if ( mask < 0 || trigger_mode < 0 )
>                      {
> +                        spin_unlock(&d->event_lock);
> +
>                          ASSERT_UNREACHABLE();
>                          return -EINVAL;

You seem to have added extra newlines between the unlock and the
return, is this intentional? I'm asking because it's not done in the
other error paths.

Roger.
diff mbox

Patch

diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
index 2fdbba6..25e3fb4 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/io.c
@@ -490,7 +490,11 @@  int pt_irq_create_bind(
             /* MSI_TRANSLATE is not supported for the hardware domain. */
             if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_PCI ||
                  pirq >= hvm_domain_irq(d)->nr_gsis )
+            {
+                spin_unlock(&d->event_lock);
+
                 return -EINVAL;
+            }
             guest_gsi = pirq;
         }
 
@@ -523,6 +527,8 @@  int pt_irq_create_bind(
 
                     if ( mask < 0 || trigger_mode < 0 )
                     {
+                        spin_unlock(&d->event_lock);
+
                         ASSERT_UNREACHABLE();
                         return -EINVAL;
                     }