From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 528C3C7EE29 for ; Sun, 28 May 2023 19:46:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231318AbjE1Tq5 (ORCPT ); Sun, 28 May 2023 15:46:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33190 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231319AbjE1Tqz (ORCPT ); Sun, 28 May 2023 15:46:55 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1F095103 for ; Sun, 28 May 2023 12:46:43 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 99ACB61F78 for ; Sun, 28 May 2023 19:46:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B6D7BC433EF; Sun, 28 May 2023 19:46:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1685303202; bh=qHJrnlh1d4xdyv6YExrX2ZBj6QAkfEOPyrfVbt3cHoA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WKe8BnP/tw1NyVsqhI/1ZS6t24qq0pkpjD01hAqV0oA6goSVw9dlPjAA6TnMCFyYe pgzYHwuYEWnawsi6N5lLvIaMDQ0qrxuWygPN0e2bX8BH6TI1NWaUTu1uGG39ybKPWt JggDyMGwEFpb8pwF1TkEsf2gNrJFiIq/dOn+4k8g= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Hans de Goede , Sebastian Reichel Subject: [PATCH 5.10 195/211] power: supply: bq27xxx: Fix bq27xxx_battery_update() race condition Date: Sun, 28 May 2023 20:11:56 +0100 Message-Id: <20230528190848.344250507@linuxfoundation.org> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230528190843.514829708@linuxfoundation.org> References: <20230528190843.514829708@linuxfoundation.org> User-Agent: quilt/0.67 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Hans de Goede commit 5c34c0aef185dcd10881847b9ebf20046aa77cb4 upstream. bq27xxx_battery_update() assumes / requires that it is only run once, not multiple times at the same time. But there are 3 possible callers: 1. bq27xxx_battery_poll() delayed_work item handler 2. bq27xxx_battery_irq_handler_thread() I2C IRQ handler 3. bq27xxx_battery_setup() And there is no protection against these racing with each other, fix this race condition by making all callers take di->lock: - Rename bq27xxx_battery_update() to bq27xxx_battery_update_unlocked() - Add new bq27xxx_battery_update() which takes di->lock and then calls bq27xxx_battery_update_unlocked() - Make stale cache check code in bq27xxx_battery_get_property(), which already takes di->lock directly to check the jiffies, call bq27xxx_battery_update_unlocked() instead of messing with the delayed_work item - Make bq27xxx_battery_update_unlocked() mod the delayed-work item so that the next poll is delayed to poll_interval milliseconds after the last update independent of the source of the update Fixes: 740b755a3b34 ("bq27x00: Poll battery state") Signed-off-by: Hans de Goede Signed-off-by: Sebastian Reichel Signed-off-by: Greg Kroah-Hartman --- drivers/power/supply/bq27xxx_battery.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) --- a/drivers/power/supply/bq27xxx_battery.c +++ b/drivers/power/supply/bq27xxx_battery.c @@ -1681,7 +1681,7 @@ static int bq27xxx_battery_read_health(s return POWER_SUPPLY_HEALTH_GOOD; } -void bq27xxx_battery_update(struct bq27xxx_device_info *di) +static void bq27xxx_battery_update_unlocked(struct bq27xxx_device_info *di) { struct bq27xxx_reg_cache cache = {0, }; bool has_ci_flag = di->opts & BQ27XXX_O_HAS_CI; @@ -1732,6 +1732,16 @@ void bq27xxx_battery_update(struct bq27x di->cache = cache; di->last_update = jiffies; + + if (poll_interval > 0) + mod_delayed_work(system_wq, &di->work, poll_interval * HZ); +} + +void bq27xxx_battery_update(struct bq27xxx_device_info *di) +{ + mutex_lock(&di->lock); + bq27xxx_battery_update_unlocked(di); + mutex_unlock(&di->lock); } EXPORT_SYMBOL_GPL(bq27xxx_battery_update); @@ -1742,9 +1752,6 @@ static void bq27xxx_battery_poll(struct work.work); bq27xxx_battery_update(di); - - if (poll_interval > 0) - schedule_delayed_work(&di->work, poll_interval * HZ); } /* @@ -1919,10 +1926,8 @@ static int bq27xxx_battery_get_property( struct bq27xxx_device_info *di = power_supply_get_drvdata(psy); mutex_lock(&di->lock); - if (time_is_before_jiffies(di->last_update + 5 * HZ)) { - cancel_delayed_work_sync(&di->work); - bq27xxx_battery_poll(&di->work.work); - } + if (time_is_before_jiffies(di->last_update + 5 * HZ)) + bq27xxx_battery_update_unlocked(di); mutex_unlock(&di->lock); if (psp != POWER_SUPPLY_PROP_PRESENT && di->cache.flags < 0)