From patchwork Mon Oct 7 10:45:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Rutland X-Patchwork-Id: 175320 Delivered-To: patch@linaro.org Received: by 2002:a92:7e96:0:0:0:0:0 with SMTP id q22csp4103331ill; Mon, 7 Oct 2019 03:45:46 -0700 (PDT) X-Google-Smtp-Source: APXvYqyyYnNsJlBBr26DataYZJMK6O+lTC3CwlBrCsLh3r4ta5IiVhFkaOM8oI8UtLoadwqTZOfD X-Received: by 2002:a50:9fce:: with SMTP id c72mr28773125edf.166.1570445146421; Mon, 07 Oct 2019 03:45:46 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1570445146; cv=none; d=google.com; s=arc-20160816; b=NrEY89KckNp/6RIbRGHZMYyuJJXAl1lUM/G2HDojSnVm4bGlQ3tg7S0jKzgPSSRymA 55JRUrSH7Vt5oH/85iVSVrlg901TNQVZtk/ZBAsRoUiGDWHfhNqswyaEvuMC3mg/HG13 CDmXucwciEzTPh6EhFSX/O1brPv1BiNYobVU9XPfDC/9Pc1vHeDiz57r59Zqp5+R3qbq oQ/HO2lJ76HzkFR+G6c9a7ehlUnZ0HHz22DFXJjT5gLxfgIXrC4sGgP6oblIQlXZuMqA 0iZKXYlpR4nKQV9pMNlF07ECMQ9um44o8RBHJJ93/IyHIUw1fcBzY0XKCGSGyC0ZWbe6 zpmw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from; bh=RY+wXBBP8+L+Fuq2ow3eDMv5KgsrN41Ci6KuIGADKlA=; b=jojgFbCQ+4rF5Z6tfiKG63+OcFtpoTzCBqaGDxE1eLrIyv9M5l2eq5t/JFvyipmVNJ K9P2XS8VAFyv75SZ/K2ohEhLKHJWU3kegUgfG7Jujonx+1rO8i+wOnk7w9/vTGfdI05j n5XusGveBNU6gRGZZe3H82RDpiv3kAzeOFrQOkg7k72jRHPDQNEgVc2S7SogOqpTeZ51 +Q9axonN+6gilbjdFHZsDFHGX+Qol0sYEywDzV8ajSdX7DvZnZ7BETE7Ck5dFMFE7YyD 2xLCVqNQLHpphaRL/y6Ix3djkQ4uOCNrXL7XWxxBBgMqYN4hIRpK8leKtDKSzptBcNuY qVfA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c9si7580569eds.211.2019.10.07.03.45.46; Mon, 07 Oct 2019 03:45:46 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727617AbfJGKpp (ORCPT + 26 others); Mon, 7 Oct 2019 06:45:45 -0400 Received: from foss.arm.com ([217.140.110.172]:59606 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727334AbfJGKpp (ORCPT ); Mon, 7 Oct 2019 06:45:45 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 38989142F; Mon, 7 Oct 2019 03:45:44 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 682263F706; Mon, 7 Oct 2019 03:45:43 -0700 (PDT) From: Mark Rutland To: linux-kernel@vger.kernel.org Cc: Mark Rutland , Marco Elver , Thomas Gleixner , Peter Zijlstra Subject: [PATCH] stop_machine: avoid potential race behaviour Date: Mon, 7 Oct 2019 11:45:36 +0100 Message-Id: <20191007104536.27276-1-mark.rutland@arm.com> X-Mailer: git-send-email 2.11.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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). Let's 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() we expect the two loads of multi_stop_data::state to be a consistent value, so we 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: Mark Rutland Cc: Marco Elver Cc: Thomas Gleixner Cc: Peter Zijlstra --- kernel/stop_machine.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) -- 2.11.0 Acked-by: Marco Elver diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c index c7031a22aa7b..998d50ee2d9b 100644 --- a/kernel/stop_machine.c +++ b/kernel/stop_machine.c @@ -7,6 +7,7 @@ * Copyright (C) 2010 SUSE Linux Products GmbH * Copyright (C) 2010 Tejun Heo */ +#include #include #include #include @@ -167,7 +168,7 @@ static void set_state(struct multi_stop_data *msdata, /* Reset ack counter. */ atomic_set(&msdata->thread_ack, msdata->num_threads); smp_wmb(); - msdata->state = newstate; + WRITE_ONCE(msdata->state, newstate); } /* Last one to ack a state moves to the next state. */ @@ -186,7 +187,7 @@ void __weak stop_machine_yield(const struct cpumask *cpumask) static int multi_cpu_stop(void *data) { struct multi_stop_data *msdata = data; - enum multi_stop_state curstate = MULTI_STOP_NONE; + enum multi_stop_state newstate, curstate = MULTI_STOP_NONE; int cpu = smp_processor_id(), err = 0; const struct cpumask *cpumask; unsigned long flags; @@ -210,8 +211,9 @@ static int multi_cpu_stop(void *data) do { /* Chill out and ensure we re-read multi_stop_state. */ stop_machine_yield(cpumask); - if (msdata->state != curstate) { - curstate = msdata->state; + newstate = READ_ONCE(msdata->state); + if (newstate != curstate) { + curstate = newstate; switch (curstate) { case MULTI_STOP_DISABLE_IRQ: local_irq_disable();