Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965057AbdGTM0B (ORCPT ); Thu, 20 Jul 2017 08:26:01 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:60206 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934456AbdGTMYQ (ORCPT ); Thu, 20 Jul 2017 08:24:16 -0400 From: Mark Brown To: Tirupathi Reddy Cc: Mark Brown , broonie@kernel.org, lgirdwood@gmail.com, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kernel@vger.kernel.org In-Reply-To: <1499851787-31016-1-git-send-email-tirupath@codeaurora.org> Message-Id: Date: Thu, 20 Jul 2017 13:24:08 +0100 X-SA-Exim-Connect-IP: 2001:470:1f1d:6b5::3 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Applied "regulator: core: fix a possible race in disable_work handling" to the regulator tree X-SA-Exim-Version: 4.2.1 (built Tue, 02 Aug 2016 21:08:31 +0000) X-SA-Exim-Scanned: No (on mezzanine.sirena.org.uk); Unknown failure Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4481 Lines: 117 The patch regulator: core: fix a possible race in disable_work handling has been applied to the regulator tree at git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From c9ccaa0cac3fc8e7d17a668aabfdf632c7c0517a Mon Sep 17 00:00:00 2001 From: Tirupathi Reddy Date: Wed, 12 Jul 2017 17:08:13 +0530 Subject: [PATCH] regulator: core: fix a possible race in disable_work handling A race condition between queueing and processing the disable_work instances results in having a work instance in the queue and the deferred_disables variable of regulator device structure having a value '0'. If no new regulator_disable_deferred() call later from clients, the deferred_disables variable value remains '0' and hits BUG() in regulator_disable_work() when the queued instance scheduled for processing the work. The race occurs as below: Core-0 Core-1 ..... /* deferred_disables = 2 */ ..... ..... /* disable_work is queued */ ..... ..... ..... regulator_disable_deferred: regulator_disable_work: mutex_lock(&rdev->mutex); ..... rdev->deferred_disables++; ..... mutex_unlock(&rdev->mutex); ..... queue_delayed_work(...) mutex_lock(&rdev->mutex); ..... count =rdev->deferred_disables; ..... rdev->deferred_disables = 0; ..... ..... ..... mutex_unlock(&rdev->mutex); ..... ..... ..... return; ..... ..... /* No new regulator_disable_deferred() calls from clients */ /* The newly queued instance is scheduled for processing */ ..... ..... regulator_disable_work: ..... mutex_lock(&rdev->mutex); BUG_ON(!rdev->deferred_disables); /* deferred_disables = 0 */ The race is fixed by removing the work instance that is queued while processing the previous queued instance. Cancel the newly queued instance from disable_work() handler just after reset the deferred_disables variable to value '0'. Also move the work queueing step before mutex_unlock in regulator_disable_deferred(). Also use mod_delayed_work() in the pace of queue_delayed_work() as queue_delayed_work() always uses the delay requested in the first call when multiple consumers call regulator_disable_deferred() close in time and does not guarantee the semantics of regulator_disable_deferred(). Signed-off-by: Tirupathi Reddy Signed-off-by: Mark Brown --- drivers/regulator/core.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index e567fa54980b..9f4d484eb25d 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -2396,6 +2396,14 @@ static void regulator_disable_work(struct work_struct *work) count = rdev->deferred_disables; rdev->deferred_disables = 0; + /* + * Workqueue functions queue the new work instance while the previous + * work instance is being processed. Cancel the queued work instance + * as the work instance under processing does the job of the queued + * work instance. + */ + cancel_delayed_work(&rdev->disable_work); + for (i = 0; i < count; i++) { ret = _regulator_disable(rdev); if (ret != 0) @@ -2439,10 +2447,10 @@ int regulator_disable_deferred(struct regulator *regulator, int ms) mutex_lock(&rdev->mutex); rdev->deferred_disables++; + mod_delayed_work(system_power_efficient_wq, &rdev->disable_work, + msecs_to_jiffies(ms)); mutex_unlock(&rdev->mutex); - queue_delayed_work(system_power_efficient_wq, &rdev->disable_work, - msecs_to_jiffies(ms)); return 0; } EXPORT_SYMBOL_GPL(regulator_disable_deferred); -- 2.13.2