From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941368AbcLVUVs (ORCPT ); Thu, 22 Dec 2016 15:21:48 -0500 Received: from terminus.zytor.com ([198.137.202.10]:42544 "EHLO terminus.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758073AbcLVUVq (ORCPT ); Thu, 22 Dec 2016 15:21:46 -0500 Date: Thu, 22 Dec 2016 12:21:06 -0800 From: tip-bot for Thomas Gleixner Message-ID: Cc: mingo@kernel.org, tglx@linutronix.de, peterz@infradead.org, hpa@zytor.com, bigeasy@linutronix.de, linux-kernel@vger.kernel.org Reply-To: tglx@linutronix.de, mingo@kernel.org, peterz@infradead.org, hpa@zytor.com, linux-kernel@vger.kernel.org, bigeasy@linutronix.de In-Reply-To: <20161221192111.675234535@linutronix.de> References: <20161221192111.675234535@linutronix.de> To: linux-tip-commits@vger.kernel.org Subject: [tip:smp/urgent] cpu/hotplug: Prevent overwriting of callbacks Git-Commit-ID: ab92ca2014a32ea5dc7f954f84661f6d96f0ba26 X-Mailer: tip-git-log-daemon Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit-ID: ab92ca2014a32ea5dc7f954f84661f6d96f0ba26 Gitweb: http://git.kernel.org/tip/ab92ca2014a32ea5dc7f954f84661f6d96f0ba26 Author: Thomas Gleixner AuthorDate: Wed, 21 Dec 2016 20:19:49 +0100 Committer: Thomas Gleixner CommitDate: Thu, 22 Dec 2016 11:37:54 +0100 cpu/hotplug: Prevent overwriting of callbacks Developers manage to overwrite states blindly without thought. That's fatal and hard to debug. Add sanity checks to make it fail. This requries to restructure the code so that the dynamic state allocation happens in the same lock protected section as the actual store. Otherwise the previous assignment of 'Reserved' to the name field would trigger the overwrite check. Signed-off-by: Thomas Gleixner Cc: Peter Zijlstra Cc: Sebastian Siewior Link: http://lkml.kernel.org/r/20161221192111.675234535@linutronix.de Signed-off-by: Thomas Gleixner --- kernel/cpu.c | 96 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 50 insertions(+), 46 deletions(-) diff --git a/kernel/cpu.c b/kernel/cpu.c index 5339aca..3ff0ea5 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -1432,23 +1432,53 @@ static int cpuhp_cb_check(enum cpuhp_state state) return 0; } -static void cpuhp_store_callbacks(enum cpuhp_state state, - const char *name, - int (*startup)(unsigned int cpu), - int (*teardown)(unsigned int cpu), - bool multi_instance) +/* + * Returns a free for dynamic slot assignment of the Online state. The states + * are protected by the cpuhp_slot_states mutex and an empty slot is identified + * by having no name assigned. + */ +static int cpuhp_reserve_state(enum cpuhp_state state) +{ + enum cpuhp_state i; + + for (i = CPUHP_AP_ONLINE_DYN; i <= CPUHP_AP_ONLINE_DYN_END; i++) { + if (!cpuhp_ap_states[i].name) + return i; + } + WARN(1, "No more dynamic states available for CPU hotplug\n"); + return -ENOSPC; +} + +static int cpuhp_store_callbacks(enum cpuhp_state state, const char *name, + int (*startup)(unsigned int cpu), + int (*teardown)(unsigned int cpu), + bool multi_instance) { /* (Un)Install the callbacks for further cpu hotplug operations */ struct cpuhp_step *sp; + int ret = 0; mutex_lock(&cpuhp_state_mutex); + + if (state == CPUHP_AP_ONLINE_DYN) { + ret = cpuhp_reserve_state(state); + if (ret < 0) + goto out; + state = ret; + } sp = cpuhp_get_step(state); + if (name && sp->name) { + ret = -EBUSY; + goto out; + } sp->startup.single = startup; sp->teardown.single = teardown; sp->name = name; sp->multi_instance = multi_instance; INIT_HLIST_HEAD(&sp->list); +out: mutex_unlock(&cpuhp_state_mutex); + return ret; } static void *cpuhp_get_teardown_cb(enum cpuhp_state state) @@ -1509,29 +1539,6 @@ static void cpuhp_rollback_install(int failedcpu, enum cpuhp_state state, } } -/* - * Returns a free for dynamic slot assignment of the Online state. The states - * are protected by the cpuhp_slot_states mutex and an empty slot is identified - * by having no name assigned. - */ -static int cpuhp_reserve_state(enum cpuhp_state state) -{ - enum cpuhp_state i; - - mutex_lock(&cpuhp_state_mutex); - for (i = CPUHP_AP_ONLINE_DYN; i <= CPUHP_AP_ONLINE_DYN_END; i++) { - if (cpuhp_ap_states[i].name) - continue; - - cpuhp_ap_states[i].name = "Reserved"; - mutex_unlock(&cpuhp_state_mutex); - return i; - } - mutex_unlock(&cpuhp_state_mutex); - WARN(1, "No more dynamic states available for CPU hotplug\n"); - return -ENOSPC; -} - int __cpuhp_state_add_instance(enum cpuhp_state state, struct hlist_node *node, bool invoke) { @@ -1580,11 +1587,13 @@ EXPORT_SYMBOL_GPL(__cpuhp_state_add_instance); /** * __cpuhp_setup_state - Setup the callbacks for an hotplug machine state - * @state: The state to setup - * @invoke: If true, the startup function is invoked for cpus where - * cpu state >= @state - * @startup: startup callback function - * @teardown: teardown callback function + * @state: The state to setup + * @invoke: If true, the startup function is invoked for cpus where + * cpu state >= @state + * @startup: startup callback function + * @teardown: teardown callback function + * @multi_instance: State is set up for multiple instances which get + * added afterwards. * * Returns: * On success: @@ -1599,25 +1608,16 @@ int __cpuhp_setup_state(enum cpuhp_state state, bool multi_instance) { int cpu, ret = 0; - int dyn_state = 0; if (cpuhp_cb_check(state) || !name) return -EINVAL; get_online_cpus(); - /* currently assignments for the ONLINE state are possible */ - if (state == CPUHP_AP_ONLINE_DYN) { - dyn_state = 1; - ret = cpuhp_reserve_state(state); - if (ret < 0) - goto out; - state = ret; - } - - cpuhp_store_callbacks(state, name, startup, teardown, multi_instance); + ret = cpuhp_store_callbacks(state, name, startup, teardown, + multi_instance); - if (!invoke || !startup) + if (ret || !invoke || !startup) goto out; /* @@ -1641,7 +1641,11 @@ int __cpuhp_setup_state(enum cpuhp_state state, } out: put_online_cpus(); - if (!ret && dyn_state) + /* + * If the requested state is CPUHP_AP_ONLINE_DYN, return the + * dynamically allocated state in case of success. + */ + if (!ret && state == CPUHP_AP_ONLINE_DYN) return state; return ret; }