• Mark Rutland's avatar
    stop_machine: Avoid potential race behaviour · b1fc5833
    Mark Rutland authored
    Both multi_cpu_stop() and set_state() access multi_stop_data::state
    racily using plain accesses. These are subject to compiler
    transformations which could break the intended behaviour of the code,
    and this situation is detected by KCSAN on both arm64 and x86 (splats
    below).
    
    Improve matters by using READ_ONCE() and WRITE_ONCE() to ensure that the
    compiler cannot elide, replay, or tear loads and stores.
    
    In multi_cpu_stop() the two loads of multi_stop_data::state are expected to
    be a consistent value, so snapshot the value into a temporary variable to
    ensure this.
    
    The state transitions are serialized by atomic manipulation of
    multi_stop_data::num_threads, and other fields in multi_stop_data are not
    modified while subject to concurrent reads.
    
    KCSAN splat on arm64:
    
    | BUG: KCSAN: data-race in multi_cpu_stop+0xa8/0x198 and set_state+0x80/0xb0
    |
    | write to 0xffff00001003bd00 of 4 bytes by task 24 on cpu 3:
    |  set_state+0x80/0xb0
    |  multi_cpu_stop+0x16c/0x198
    |  cpu_stopper_thread+0x170/0x298
    |  smpboot_thread_fn+0x40c/0x560
    |  kthread+0x1a8/0x1b0
    |  ret_from_fork+0x10/0x18
    |
    | read to 0xffff00001003bd00 of 4 bytes by task 14 on cpu 1:
    |  multi_cpu_stop+0xa8/0x198
    |  cpu_stopper_thread+0x170/0x298
    |  smpboot_thread_fn+0x40c/0x560
    |  kthread+0x1a8/0x1b0
    |  ret_from_fork+0x10/0x18
    |
    | Reported by Kernel Concurrency Sanitizer on:
    | CPU: 1 PID: 14 Comm: migration/1 Not tainted 5.3.0-00007-g67ab35a199f4-dirty #3
    | Hardware name: linux,dummy-virt (DT)
    
    KCSAN splat on x86:
    
    | write to 0xffffb0bac0013e18 of 4 bytes by task 19 on cpu 2:
    |  set_state kernel/stop_machine.c:170 [inline]
    |  ack_state kernel/stop_machine.c:177 [inline]
    |  multi_cpu_stop+0x1a4/0x220 kernel/stop_machine.c:227
    |  cpu_stopper_thread+0x19e/0x280 kernel/stop_machine.c:516
    |  smpboot_thread_fn+0x1a8/0x300 kernel/smpboot.c:165
    |  kthread+0x1b5/0x200 kernel/kthread.c:255
    |  ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:352
    |
    | read to 0xffffb0bac0013e18 of 4 bytes by task 44 on cpu 7:
    |  multi_cpu_stop+0xb4/0x220 kernel/stop_machine.c:213
    |  cpu_stopper_thread+0x19e/0x280 kernel/stop_machine.c:516
    |  smpboot_thread_fn+0x1a8/0x300 kernel/smpboot.c:165
    |  kthread+0x1b5/0x200 kernel/kthread.c:255
    |  ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:352
    |
    | Reported by Kernel Concurrency Sanitizer on:
    | CPU: 7 PID: 44 Comm: migration/7 Not tainted 5.3.0+ #1
    | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
    Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
    Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Acked-by: default avatarMarco Elver <elver@google.com>
    Link: https://lkml.kernel.org/r/20191007104536.27276-1-mark.rutland@arm.com
    b1fc5833
stop_machine.c 18 KB