From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6C5AA550DA6; Wed, 9 Sep 2026 13:47:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788961624; cv=none; b=CDEK+wwzPreQxy0gEY1nsD8w+A096wfD4gUcGyootZyMCBtZ18EW8VxngvpWrm9IG6r2ojeHEGoN4V7ZzpF0cxhZfZOf3c6J+nxvEpqczwZsjwbup6+mvuMgFNGGCbgiGl1x4fgRU3FfB/2B0NoI1ar8yOGx3cXTTSdj6oLpn0Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788961624; c=relaxed/simple; bh=mZaW7k8+EEdhI1g+OWrIrMksxmX3lYApKuZtxyk4/ZQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=CqQZ2a7/ujyjaJBbbUeQWkeW97xEA8QlFUFjlkkqwhomj5GV4Ub0ZqpvNOHvijoJalhA1FKwQkqjbcMGlxEQDMhwNzAg7A4J1PMc341aU2EIUN581ykGNbNAaCwzI+03w4Q0LS05958RAZE33554pdJo3BrweZnqYykUq4DN5JA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=gXyw9k2z; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="gXyw9k2z" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6DB031F00A3A; Wed, 9 Sep 2026 13:47:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxfoundation.org; s=korg; t=1788961622; bh=XtnNQrfulvr0P5RxNmqpSTJiXBH2hmMMh2DptIrIG7E=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=gXyw9k2z/8O5l0xL0cueJify9elgZmcRYadXzpHCQf9Au8fnjp7etWRL+yUdEbHdH tl6DS/lTOvFuXHh4R3liYVEto5aca8mN26dps6IzYYjdGUw6ORJt6sOhqBhcEpgDgy ffqEsDPxg5pC1+OV+D0wXDluJpoqgGRWKTgwbPa4= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Rick , Avraham Hollander , Rong Zhang , "Rafael J. Wysocki" , Sasha Levin Subject: [PATCH 7.2 013/556] ACPI: battery: Protect all properties with a separated mutex Date: Wed, 9 Sep 2026 15:34:53 +0200 Message-ID: <20260909134230.940012440@linuxfoundation.org> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260909134230.441546314@linuxfoundation.org> References: <20260909134230.441546314@linuxfoundation.org> User-Agent: quilt/0.69 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 7.2-stable review patch. If anyone has any objections, please let me know. ------------------ From: Rong Zhang [ Upstream commit 9e409f1dff7841634e4b101111d6427f979c0aac ] The acpi_battery_get_property() callback calls acpi_battery_get_state() without any lock held. On some devices, it happens that the property cache has expired before a uevent reaches userspace, triggering simultaneous attempts to evaluate _BST. See [1] for an analysis to sysrq stacktraces on one of the these devices. In a few cases, including when the AML is sleeping or acquiring a mutex, ACPICA drops the namespace and interpreter locks and allows the evaluation of _BST to start while another task is still evaluating it. This could somehow confuse the interpreter and lead to chaos in AML mutexes on some devices, see [2] for an example. Not holding the lock is also prone to race conditions, for example: CPU0 | CPU1 acpi_battery_get_property() | acpi_battery_get_state() | [update_time expired] | extract_package() | acpi_battery_get_property() battery->update_time = jiffies | acpi_battery_get_state() kfree() | [up to date] | [read capacity_now] [fix capacity_now due to quirk] | where CPU1 gets raw capacity_now before CPU0 fixes it to a meaningful value. The existing mutex update_lock is not applicapable for acpi_battery_get_property(), as some code path could call or wait for acpi_battery_get_property() while holding update_lock. Therefore, introduce a mutex called property_lock to protect all accesses to battery properties, so that acpi_battery_get_property() can take the advantage of the mutex and synchronize itself. With the mutex, acpi_battery_get_state() are synchronized in all code paths calling it, and its cache mechanism can always clamp the frequency of _BST evaluations according to cache_time. The helper function acpi_battery_handle_discharging() for quirky devices has to be inlined due to the change, as the mutex must be unlocked before calling the expensive power_supply_is_system_supplied() helper function. Fixes: 86bfd21a0baf ("ACPI: battery: Drop redundant locking") Reported-by: Rick Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221065#c85 [1] Reported-by: Avraham Hollander Tested-by: Avraham Hollander Closes: https://lore.kernel.org/linux-acpi/CAP1mzZReJCn6df5DwEPu-JCQUyr=Pu1cg5xKCMttWZkHCQtVmQ@mail.gmail.com [2] Signed-off-by: Rong Zhang Cc: All applicable Link: https://patch.msgid.link/20260809-b4-acpi-battery-notification-v5-1-788d54fa2e35@rong.moe Signed-off-by: Rafael J. Wysocki Signed-off-by: Sasha Levin Signed-off-by: Greg Kroah-Hartman --- drivers/acpi/battery.c | 147 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 101 insertions(+), 46 deletions(-) --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -97,6 +98,9 @@ struct acpi_battery { struct device *phys_dev; struct notifier_block pm_nb; struct list_head list; + unsigned long flags; + + struct mutex property_lock; /* Protects properties below. */ unsigned long update_time; int revision; int rate_now; @@ -123,7 +127,6 @@ struct acpi_battery { char oem_info[MAX_STRING_LENGTH]; int state; int power_unit; - unsigned long flags; }; #define to_acpi_battery(x) power_supply_get_drvdata(x) @@ -180,20 +183,6 @@ static bool acpi_battery_is_degraded(str battery->full_charge_capacity < battery->design_capacity; } -static int acpi_battery_handle_discharging(struct acpi_battery *battery) -{ - /* - * Some devices wrongly report discharging if the battery's charge level - * was above the device's start charging threshold atm the AC adapter - * was plugged in and the device thus did not start a new charge cycle. - */ - if ((battery_ac_is_broken || power_supply_is_system_supplied()) && - battery->rate_now == 0) - return POWER_SUPPLY_STATUS_NOT_CHARGING; - - return POWER_SUPPLY_STATUS_DISCHARGING; -} - static int acpi_battery_get_property(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val) @@ -201,15 +190,41 @@ static int acpi_battery_get_property(str int full_capacity = ACPI_BATTERY_VALUE_UNKNOWN, ret = 0; struct acpi_battery *battery = to_acpi_battery(psy); - if (acpi_battery_present(battery)) { - /* run battery update only if it is present */ - acpi_battery_get_state(battery); - } else if (psp != POWER_SUPPLY_PROP_PRESENT) - return -ENODEV; + /* run battery update only if it is present */ + if (!acpi_battery_present(battery)) { + switch (psp) { + case POWER_SUPPLY_PROP_PRESENT: + val->intval = 0; + return 0; + default: + return -ENODEV; + } + } + + mutex_lock(&battery->property_lock); + + acpi_battery_get_state(battery); + switch (psp) { case POWER_SUPPLY_PROP_STATUS: + /* + * Some devices wrongly report discharging if the battery's charge level + * was above the device's start charging threshold atm the AC adapter + * was plugged in and the device thus did not start a new charge cycle. + */ if (battery->state & ACPI_BATTERY_STATE_DISCHARGING) - val->intval = acpi_battery_handle_discharging(battery); + if (battery->rate_now != 0) { + val->intval = POWER_SUPPLY_STATUS_DISCHARGING; + } else if (battery_ac_is_broken) { + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; + } else { + mutex_unlock(&battery->property_lock); + + val->intval = power_supply_is_system_supplied() + ? POWER_SUPPLY_STATUS_NOT_CHARGING + : POWER_SUPPLY_STATUS_DISCHARGING; + return 0; + } else if (battery->state & ACPI_BATTERY_STATE_CHARGING) /* Validate the status by checking the current. */ if (battery->rate_now != ACPI_BATTERY_VALUE_UNKNOWN && @@ -311,6 +326,8 @@ static int acpi_battery_get_property(str default: ret = -EINVAL; } + + mutex_unlock(&battery->property_lock); return ret; } @@ -533,6 +550,8 @@ static int acpi_battery_get_info(struct int use_bix; int result = -ENODEV; + lockdep_assert_held(&battery->property_lock); + if (!acpi_battery_present(battery)) return 0; @@ -572,6 +591,8 @@ static int acpi_battery_get_state(struct acpi_status status = 0; struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; + lockdep_assert_held(&battery->property_lock); + if (!acpi_battery_present(battery)) return 0; @@ -625,6 +646,8 @@ static int acpi_battery_set_alarm(struct { acpi_status status = 0; + lockdep_assert_held(&battery->property_lock); + if (!acpi_battery_present(battery) || !test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags)) return -ENODEV; @@ -642,6 +665,8 @@ static int acpi_battery_set_alarm(struct static int acpi_battery_init_alarm(struct acpi_battery *battery) { + lockdep_assert_held(&battery->property_lock); + /* See if alarms are supported, and if so, set default */ if (!acpi_has_method(battery->device->handle, "_BTP")) { clear_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags); @@ -659,6 +684,8 @@ static ssize_t acpi_battery_alarm_show(s { struct acpi_battery *battery = to_acpi_battery(dev_get_drvdata(dev)); + guard(mutex)(&battery->property_lock); + return sysfs_emit(buf, "%d\n", battery->alarm * 1000); } @@ -674,6 +701,8 @@ static ssize_t acpi_battery_alarm_store( if (err) return err; + guard(mutex)(&battery->property_lock); + battery->alarm = x / 1000; if (acpi_battery_present(battery)) acpi_battery_set_alarm(battery); @@ -858,12 +887,17 @@ static int sysfs_add_battery(struct acpi .no_wakeup_source = true, }; bool full_cap_broken = false; + int power_unit; - if (!ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity) && - !ACPI_BATTERY_CAPACITY_VALID(battery->design_capacity)) - full_cap_broken = true; + scoped_guard(mutex, &battery->property_lock) { + power_unit = battery->power_unit; - if (battery->power_unit == ACPI_BATTERY_POWER_UNIT_MA) { + if (!ACPI_BATTERY_CAPACITY_VALID(battery->full_charge_capacity) && + !ACPI_BATTERY_CAPACITY_VALID(battery->design_capacity)) + full_cap_broken = true; + } + + if (power_unit == ACPI_BATTERY_POWER_UNIT_MA) { if (full_cap_broken) { battery->bat_desc.properties = charge_battery_full_cap_broken_props; @@ -917,6 +951,9 @@ static void sysfs_remove_battery(struct static void find_battery(const struct dmi_header *dm, void *private) { struct acpi_battery *battery = (struct acpi_battery *)private; + + lockdep_assert_held(&battery->property_lock); + /* Note: the hardcoded offsets below have been extracted from * the source code of dmidecode. */ @@ -948,6 +985,8 @@ static void find_battery(const struct dm */ static void acpi_battery_quirks(struct acpi_battery *battery) { + lockdep_assert_held(&battery->property_lock); + if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags)) return; @@ -1000,30 +1039,38 @@ static void acpi_battery_quirks(struct a static int acpi_battery_update(struct acpi_battery *battery, bool resume) { int result = acpi_battery_get_status(battery); + bool wakeup; if (result) return result; if (!acpi_battery_present(battery)) { sysfs_remove_battery(battery); - battery->update_time = 0; + scoped_guard(mutex, &battery->property_lock) + battery->update_time = 0; return 0; } if (resume) return 0; - if (!battery->update_time) { - result = acpi_battery_get_info(battery); + scoped_guard(mutex, &battery->property_lock) { + if (!battery->update_time) { + result = acpi_battery_get_info(battery); + if (result) + return result; + acpi_battery_init_alarm(battery); + } + + result = acpi_battery_get_state(battery); if (result) return result; - acpi_battery_init_alarm(battery); - } + acpi_battery_quirks(battery); - result = acpi_battery_get_state(battery); - if (result) - return result; - acpi_battery_quirks(battery); + wakeup = ((battery->state & ACPI_BATTERY_STATE_CRITICAL) || + (test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags) && + (battery->capacity_now <= battery->alarm))); + } if (!battery->bat) { result = sysfs_add_battery(battery); @@ -1035,9 +1082,7 @@ static int acpi_battery_update(struct ac * Wakeup the system if battery is critical low * or lower than the alarm level */ - if ((battery->state & ACPI_BATTERY_STATE_CRITICAL) || - (test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags) && - (battery->capacity_now <= battery->alarm))) + if (wakeup) acpi_pm_wakeup_event(battery->phys_dev); return result; @@ -1050,12 +1095,14 @@ static void acpi_battery_refresh(struct if (!battery->bat) return; - power_unit = battery->power_unit; + scoped_guard(mutex, &battery->property_lock) { + power_unit = battery->power_unit; - acpi_battery_get_info(battery); + acpi_battery_get_info(battery); - if (power_unit == battery->power_unit) - return; + if (power_unit == battery->power_unit) + return; + } /* The battery has changed its reporting units. */ sysfs_remove_battery(battery); @@ -1110,17 +1157,21 @@ static int battery_notify(struct notifie } else { int result; - result = acpi_battery_get_info(battery); - if (result) - return result; + scoped_guard(mutex, &battery->property_lock) { + result = acpi_battery_get_info(battery); + if (result) + return result; + } result = sysfs_add_battery(battery); if (result) return result; } - acpi_battery_init_alarm(battery); - acpi_battery_get_state(battery); + scoped_guard(mutex, &battery->property_lock) { + acpi_battery_init_alarm(battery); + acpi_battery_get_state(battery); + } } return 0; @@ -1262,6 +1313,10 @@ static int acpi_battery_probe(struct pla if (result) return result; + result = devm_mutex_init(&pdev->dev, &battery->property_lock); + if (result) + return result; + if (acpi_has_method(battery->device->handle, "_BIX")) set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);