From patchwork Tue Oct 14 12:20:39 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Krzysztof Kozlowski X-Patchwork-Id: 5079931 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C1E18C11AC for ; Tue, 14 Oct 2014 12:25:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9F78420127 for ; Tue, 14 Oct 2014 12:25:00 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6C73520179 for ; Tue, 14 Oct 2014 12:24:59 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xe174-00031z-6h; Tue, 14 Oct 2014 12:22:02 +0000 Received: from mailout4.w1.samsung.com ([210.118.77.14]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xe16T-0002jy-LU for linux-arm-kernel@lists.infradead.org; Tue, 14 Oct 2014 12:21:28 +0000 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NDF00MS2P3OXJ20@mailout4.w1.samsung.com> for linux-arm-kernel@lists.infradead.org; Tue, 14 Oct 2014 13:23:48 +0100 (BST) X-AuditID: cbfec7f5-b7f776d000003e54-a1-543d152d9e1c Received: from eusync3.samsung.com ( [203.254.199.213]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id A3.63.15956.D251D345; Tue, 14 Oct 2014 13:21:01 +0100 (BST) Received: from AMDC1943.digital.local ([106.116.151.171]) by eusync3.samsung.com (Oracle Communications Messaging Server 7u4-23.01(7.0.4.23.0) 64bit (built Aug 10 2011)) with ESMTPA id <0NDF009UZOYWAA30@eusync3.samsung.com>; Tue, 14 Oct 2014 13:21:00 +0100 (BST) From: Krzysztof Kozlowski To: Linus Walleij , Samuel Ortiz , Lee Jones , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org Subject: [PATCH 1/8] power_supply: Add API for safe access of power supply function attrs Date: Tue, 14 Oct 2014 14:20:39 +0200 Message-id: <1413289246-31650-2-git-send-email-k.kozlowski@samsung.com> X-Mailer: git-send-email 1.9.1 In-reply-to: <1413289246-31650-1-git-send-email-k.kozlowski@samsung.com> References: <1413289246-31650-1-git-send-email-k.kozlowski@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrKLMWRmVeSWpSXmKPExsVy+t/xq7q6orYhBmu+MVoc3KppsXHGelaL SU/eM1tMXDmZ2aLz7BNmi9cvDC3ONr1ht7j/9SijxZQ/y5ksNj2+xmpxedccNovPvUcYLZ4s PMNksfbIXXaL240r2CzunjrKZnG6m9Xi9O4SByGPCf2fGD12zrrL7rF5hZbHplWdbB53ru1h 85h3MtBj85J6j53fG9g9+rasYvRYsfo7u8fnTXIB3FFcNimpOZllqUX6dglcGbO3fWEp+OxW 8XDNPOYGxtU2XYycHBICJhJvJt1hg7DFJC7cWw9kc3EICSxllFj7qIUZwuljknjw7Rk7SBWb gLHE5uVLwKpEBK4zSXSumMAC4jALNDJLvGtoZupi5OAQFoiW+H8gBqSBRUBV4nTTI7BmXgF3 ibUfV7JCrJOTOHlsMpjNKeAhsXLRERYQWwiopv3kbbYJjLwLGBlWMYqmliYXFCel5xrpFSfm Fpfmpesl5+duYoQE/dcdjEuPWR1iFOBgVOLhLYi0CRFiTSwrrsw9xCjBwawkwqvAYRsixJuS WFmVWpQfX1Sak1p8iJGJg1OqgXF9ZXT0awcXiU3JarXhN3x4j5/gPy3dNDPBSIh1bpJH2r0r nkvX87GZXVVROJyRulxw9YwH++4tW8xzJ/3XtnKfTtusuW4WvYceTr6x6Z2Ur736FUchycS1 7BcDV/4KsWlp+mj3fcnto2kBiz6FJycUiXzaV7JvbXjNhLcmN5ycLy05dSHylooSS3FGoqEW c1FxIgAOkdPSWAIAAA== X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141014_052125_897350_A9CACC92 X-CRM114-Status: GOOD ( 14.65 ) X-Spam-Score: -5.0 (-----) Cc: Jonghwa Lee , Bartlomiej Zolnierkiewicz , Anton Vorontsov , Krzysztof Kozlowski , Kyungmin Park , Myungjoo Ham , Pavel Machek , Guenter Roeck , Marek Szyprowski X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Add simple wrappers for accessing power supply's function attributes: - get_property -> power_supply_get_property - set_property -> power_supply_set_property - property_is_writeable -> power_supply_property_is_writeable - external_power_changed -> power_supply_external_power_changed - set_charged -> power_supply_set_charged This API adds a safe way of accessing a power supply from another driver. Particularly it solves invalid memory references (and lockup) in following race condition scenario: Thread 1: charger manager Thread 2: power supply driver, used by charger manager THREAD 1 (charger manager) THREAD 2 (power supply driver) Acked-by: Pavel Machek ========================== ============================== psy = power_supply_get_by_name() Driver unbind, .remove power_supply_unregister Device fully removed psy->get_property() The 'get_property' callback is still valid and leads to actual calling of Thread2->get_property() but now in invalid context because the driver was unbound. This could be observed easily with charger manager driver (here compiled with max17042 fuel gauge): $ echo "12-0036" > /sys/bus/i2c/drivers/max17042/unbind $ cat /sys/devices/virtual/power_supply/battery/temp [ 240.505998] INFO: task cat:1394 blocked for more than 120 seconds. [ 240.510762] Not tainted 3.17.0-next-20141007-00028-ge60b6dd79570-dirty #183 [ 240.518208] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 240.526025] cat D c0469548 0 1394 1 0x00000000 [ 240.532384] [] (__schedule) from [] (schedule_preempt_disabled+0x14/0x20) [ 240.540885] [] (schedule_preempt_disabled) from [] (mutex_lock_nested+0x1bc/0x458) [ 240.550171] [] (mutex_lock_nested) from [] (regmap_read+0x30/0x60) [ 240.558079] [] (regmap_read) from [] (max17042_get_property+0x2e8/0x350) [ 240.566490] [] (max17042_get_property) from [] (charger_get_property+0x238/0x31c) [ 240.575688] [] (charger_get_property) from [] (power_supply_show_property+0x48/0x1e0) [ 240.585241] [] (power_supply_show_property) from [] (dev_attr_show+0x1c/0x48) [ 240.594100] [] (dev_attr_show) from [] (sysfs_kf_seq_show+0x84/0x104) [ 240.602251] [] (sysfs_kf_seq_show) from [] (kernfs_seq_show+0x24/0x28) [ 240.610497] [] (kernfs_seq_show) from [] (seq_read+0x1b0/0x484) [ 240.618138] [] (seq_read) from [] (vfs_read+0x88/0x144) [ 240.625127] [] (vfs_read) from [] (SyS_read+0x40/0x8c) [ 240.631941] [] (SyS_read) from [] (ret_fast_syscall+0x0/0x48) Or: [ 17.277605] Division by zero in kernel. [ 17.280043] CPU: 2 PID: 1384 Comm: cat Not tainted 3.17.0-next-20141007-00028-ge60b6dd79570-dirty #181 [ 17.289348] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 17.297055] [] (show_stack) from [] (dump_stack+0x70/0xbc) [ 17.304264] [] (dump_stack) from [] (Ldiv0+0x8/0x10) [ 17.310933] [] (Ldiv0) from [] (__aeabi_uidivmod+0x8/0x18) [ 17.318132] [] (__aeabi_uidivmod) from [] (regmap_read+0x1c/0x60) [ 17.325956] [] (regmap_read) from [] (max17042_get_property+0x1bc/0x350) [ 17.334361] [] (max17042_get_property) from [] (charger_get_property+0x198/0x328) [ 17.343560] [] (charger_get_property) from [] (power_supply_show_property+0x48/0x1e0) [ 17.353108] [] (power_supply_show_property) from [] (power_supply_uevent+0x9c/0x1c4) [ 17.362575] [] (power_supply_uevent) from [] (dev_uevent+0xb8/0x1c8) [ 17.370639] [] (dev_uevent) from [] (uevent_show+0xa8/0x104) [ 17.378015] [] (uevent_show) from [] (dev_attr_show+0x1c/0x48) [ 17.385579] [] (dev_attr_show) from [] (sysfs_kf_seq_show+0x84/0x104) [ 17.393731] [] (sysfs_kf_seq_show) from [] (kernfs_seq_show+0x24/0x28) [ 17.401979] [] (kernfs_seq_show) from [] (seq_read+0x1b0/0x484) [ 17.409619] [] (seq_read) from [] (vfs_read+0x88/0x144) [ 17.416557] [] (vfs_read) from [] (SyS_read+0x40/0x8c) [ 17.423416] [] (SyS_read) from [] (ret_fast_syscall+0x0/0x48) [ 17.430880] power_supply battery: driver failed to report `voltage_now' property: -22 Signed-off-by: Krzysztof Kozlowski --- drivers/power/power_supply_core.c | 57 +++++++++++++++++++++++++++++++++++++-- include/linux/power_supply.h | 16 +++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c index 6cb7fe5c022d..8e36986a73e1 100644 --- a/drivers/power/power_supply_core.c +++ b/drivers/power/power_supply_core.c @@ -366,6 +366,56 @@ struct power_supply *power_supply_get_by_phandle(struct device_node *np, EXPORT_SYMBOL_GPL(power_supply_get_by_phandle); #endif /* CONFIG_OF */ +int power_supply_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + if (!psy || !psy->dev) + return -ENODEV; + + return psy->get_property(psy, psp, val); +} +EXPORT_SYMBOL_GPL(power_supply_get_property); + +int power_supply_set_property(struct power_supply *psy, + enum power_supply_property psp, + const union power_supply_propval *val) +{ + if (!psy || !psy->dev || !psy->set_property) + return -ENODEV; + + return psy->set_property(psy, psp, val); +} +EXPORT_SYMBOL_GPL(power_supply_set_property); + +int power_supply_property_is_writeable(struct power_supply *psy, + enum power_supply_property psp) +{ + if (!psy || !psy->dev || !psy->property_is_writeable) + return -ENODEV; + + return psy->property_is_writeable(psy, psp); +} +EXPORT_SYMBOL_GPL(power_supply_property_is_writeable); + +void power_supply_external_power_changed(struct power_supply *psy) +{ + if (!psy || !psy->dev || !psy->external_power_changed) + return; + + psy->external_power_changed(psy); +} +EXPORT_SYMBOL_GPL(power_supply_external_power_changed); + +void power_supply_set_charged(struct power_supply *psy) +{ + if (!psy || !psy->dev || !psy->set_charged) + return; + + psy->set_charged(psy); +} +EXPORT_SYMBOL_GPL(power_supply_set_charged); + int power_supply_powers(struct power_supply *psy, struct device *dev) { return sysfs_create_link(&psy->dev->kobj, &dev->kobj, "powers"); @@ -616,13 +666,16 @@ EXPORT_SYMBOL_GPL(power_supply_register_no_ws); void power_supply_unregister(struct power_supply *psy) { + struct device *dev = psy->dev; + cancel_work_sync(&psy->changed_work); sysfs_remove_link(&psy->dev->kobj, "powers"); + psy->dev = NULL; power_supply_remove_triggers(psy); psy_unregister_cooler(psy); psy_unregister_thermal(psy); - device_init_wakeup(psy->dev, false); - device_unregister(psy->dev); + device_init_wakeup(dev, false); + device_unregister(dev); } EXPORT_SYMBOL_GPL(power_supply_unregister); diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 3ed049673022..44e749c65505 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -189,6 +189,12 @@ struct power_supply { size_t num_supplies; struct device_node *of_node; + /* + * Functions for drivers implementing power supply class. + * These shouldn't be called directly by other drivers for accessing + * this power supply. Instead use power_supply_*() functions (for + * example power_supply_get_property()). + */ int (*get_property)(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val); @@ -268,6 +274,16 @@ extern int power_supply_is_system_supplied(void); static inline int power_supply_is_system_supplied(void) { return -ENOSYS; } #endif +extern int power_supply_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val); +extern int power_supply_set_property(struct power_supply *psy, + enum power_supply_property psp, + const union power_supply_propval *val); +extern int power_supply_property_is_writeable(struct power_supply *psy, + enum power_supply_property psp); +extern void power_supply_external_power_changed(struct power_supply *psy); +extern void power_supply_set_charged(struct power_supply *psy); extern int power_supply_register(struct device *parent, struct power_supply *psy); extern int power_supply_register_no_ws(struct device *parent,