In case moving away all domains from the cpu to be removed is failing
in cpupool_unassign_cpu_start() the error path is missing to release
sched_res_rculock.

The normal exit path is releasing domlist_read_lock instead (this is
currently no problem as the reference to the specific rcu lock is not
used by rcu_read_unlock()).

Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/common/sched/cpupool.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched/cpupool.c b/xen/common/sched/cpupool.c
index b7721b5d02..4bf3d8434d 100644
--- a/xen/common/sched/cpupool.c
+++ b/xen/common/sched/cpupool.c
@@ -466,7 +466,7 @@ static int cpupool_unassign_cpu_start(struct cpupool *c, unsigned int cpu)
         }
         rcu_read_unlock(&domlist_read_lock);
         if ( ret )
-            goto out;
+            goto out_rcu;
     }
     cpupool_moving_cpu = cpu;
     atomic_inc(&c->refcnt);
@@ -474,7 +474,8 @@ static int cpupool_unassign_cpu_start(struct cpupool *c, unsigned int cpu)
     cpumask_andnot(c->cpu_valid, c->cpu_valid, cpus);
     cpumask_and(c->res_valid, c->cpu_valid, &sched_res_mask);
 
-    rcu_read_unlock(&domlist_read_lock);
+out_rcu:
+    rcu_read_unlock(&sched_res_rculock);
 out:
     spin_unlock(&cpupool_lock);
 
-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 03.03.2020 09:03, Juergen Gross wrote:
> In case moving away all domains from the cpu to be removed is failing
> in cpupool_unassign_cpu_start() the error path is missing to release
> sched_res_rculock.
> 
> The normal exit path is releasing domlist_read_lock instead (this is
> currently no problem as the reference to the specific rcu lock is not
> used by rcu_read_unlock()).
> 
> Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>

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

> --- a/xen/common/sched/cpupool.c
> +++ b/xen/common/sched/cpupool.c
> @@ -466,7 +466,7 @@ static int cpupool_unassign_cpu_start(struct cpupool *c, unsigned int cpu)
>          }
>          rcu_read_unlock(&domlist_read_lock);
>          if ( ret )
> -            goto out;
> +            goto out_rcu;
>      }
>      cpupool_moving_cpu = cpu;
>      atomic_inc(&c->refcnt);
> @@ -474,7 +474,8 @@ static int cpupool_unassign_cpu_start(struct cpupool *c, unsigned int cpu)
>      cpumask_andnot(c->cpu_valid, c->cpu_valid, cpus);
>      cpumask_and(c->res_valid, c->cpu_valid, &sched_res_mask);
>  
> -    rcu_read_unlock(&domlist_read_lock);
> +out_rcu:

... the label indented by at least one blank, at which point it
may make sense to also ...

> +    rcu_read_unlock(&sched_res_rculock);
>  out:

... correct this one.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel