diff mbox series

[1/3] xen/cpu: Distinguish "cpu already in that state" in cpu_{up, down}()

Message ID 1554235048-3373-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86/smt: Runtime SMT controls | expand

Commit Message

Andrew Cooper April 2, 2019, 7:57 p.m. UTC 
All methods of querying the online state of a CPU are racy without the hotplug
lock held, which can lead to a TOCTOU race trying to online or offline CPUs.

Distinguish this case with -EEXIST rather than -EINVAL, so the caller can take
other actions if necessary.

While adjusting this, rework the code slightly to fold the exit paths, which
results in a minor reduction in compiled code size.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/common/cpu.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

Comments

Jan Beulich April 3, 2019, 8:49 a.m. UTC | #1 
>>> On 02.04.19 at 21:57, <andrew.cooper3@citrix.com> wrote:
> All methods of querying the online state of a CPU are racy without the hotplug
> lock held, which can lead to a TOCTOU race trying to online or offline CPUs.
> 
> Distinguish this case with -EEXIST rather than -EINVAL, so the caller can take
> other actions if necessary.
> 
> While adjusting this, rework the code slightly to fold the exit paths, which
> results in a minor reduction in compiled code size.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Patch

diff --git a/xen/common/cpu.c b/xen/common/cpu.c
index 836c62f..1829318 100644
--- a/xen/common/cpu.c
+++ b/xen/common/cpu.c
@@ -94,11 +94,13 @@  int cpu_down(unsigned int cpu)
     if ( !cpu_hotplug_begin() )
         return -EBUSY;
 
-    if ( (cpu >= nr_cpu_ids) || (cpu == 0) || !cpu_online(cpu) )
-    {
-        cpu_hotplug_done();
-        return -EINVAL;
-    }
+    err = -EINVAL;
+    if ( (cpu >= nr_cpu_ids) || (cpu == 0) )
+        goto out;
+
+    err = -EEXIST;
+    if ( !cpu_online(cpu) )
+        goto out;
 
     notifier_rc = notifier_call_chain(&cpu_chain, CPU_DOWN_PREPARE, hcpu, &nb);
     if ( notifier_rc != NOTIFY_DONE )
@@ -125,6 +127,7 @@  int cpu_down(unsigned int cpu)
  fail:
     notifier_rc = notifier_call_chain(&cpu_chain, CPU_DOWN_FAILED, hcpu, &nb);
     BUG_ON(notifier_rc != NOTIFY_DONE);
+ out:
     cpu_hotplug_done();
     return err;
 }
@@ -138,11 +141,13 @@  int cpu_up(unsigned int cpu)
     if ( !cpu_hotplug_begin() )
         return -EBUSY;
 
-    if ( (cpu >= nr_cpu_ids) || cpu_online(cpu) || !cpu_present(cpu) )
-    {
-        cpu_hotplug_done();
-        return -EINVAL;
-    }
+    err = -EINVAL;
+    if ( (cpu >= nr_cpu_ids) || !cpu_present(cpu) )
+        goto out;
+
+    err = -EEXIST;
+    if ( cpu_online(cpu) )
+        goto out;
 
     notifier_rc = notifier_call_chain(&cpu_chain, CPU_UP_PREPARE, hcpu, &nb);
     if ( notifier_rc != NOTIFY_DONE )
@@ -166,6 +171,7 @@  int cpu_up(unsigned int cpu)
  fail:
     notifier_rc = notifier_call_chain(&cpu_chain, CPU_UP_CANCELED, hcpu, &nb);
     BUG_ON(notifier_rc != NOTIFY_DONE);
+ out:
     cpu_hotplug_done();
     return err;
 }