LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Sebastian Reichel <sre@kernel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Len Brown <lenb@kernel.org>, Jiri Kosina <jkosina@suse.cz>,
	David Herrmann <dh.herrmann@googlemail.com>,
	Cezary Jackiewicz <cezary.jackiewicz@gmail.com>,
	Darren Hart <dvhart@infradead.org>,
	Support Opensource <support.opensource@diasemi.com>,
	Milo Kim <milo.kim@ti.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Samuel Ortiz <sameo@linux.intel.com>,
	Lee Jones <lee.jones@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-acpi@vger.kernel.org, linux-input@vger.kernel.org,
	platform-driver-x86@vger.kernel.org,
	patches@opensource.wolfsonmicro.com, ac100@lists.launchpad.net,
	linux-tegra@vger.kernel.org, devel@driverdev.osuosl.org,
	Thomas Gleixner <tglx@linutronix.de>, Pavel Machek <pavel@ucw.cz>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH v6 13/22] power_supply: Change ownership from driver to core
Date: Fri, 13 Mar 2015 00:11:45 +0100	[thread overview]
Message-ID: <1926782.hW6KizxuR8@vostro.rjw.lan> (raw)
In-Reply-To: <1425976046-20973-14-git-send-email-k.kozlowski@samsung.com>

On Tuesday, March 10, 2015 09:27:17 AM Krzysztof Kozlowski wrote:
> Change the ownership of power_supply structure from each driver
> implementing the class to the power supply core.
> 
> The patch changes power_supply_register() function thus all drivers
> implementing power supply class are adjusted.
> 
> Each driver provides the implementation of power supply. However it
> should not be the owner of power supply class instance because it is
> exposed by core to other subsystems with power_supply_get_by_name().
> These other subsystems have no knowledge when the driver will unregister
> the power supply. This leads to several issues when driver is unbound -
> mostly because user of power supply accesses freed memory.
> 
> Instead let the core own the instance of struct 'power_supply'.  Other
> users of this power supply will still access valid memory because it
> will be freed when device reference count reaches 0. Currently this
> means "it will leak" but power_supply_put() call in next patches will
> solve it.
> 
> This solves invalid memory references 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)
> ==========================         ==============================
> psy = power_supply_get_by_name()
>                                    Driver unbind, .remove
>                                      power_supply_unregister()
>                                      Device fully removed
> psy->get_property()
> 
> The 'get_property' call is executed in invalid context because the driver was
> unbound and struct 'power_supply' memory was freed.
> 
> This could be observed easily with charger manager driver (here compiled
> with max17040 fuel gauge):
> 
> $ cat /sys/devices/virtual/power_supply/cm-battery/capacity &
> $ echo "1-0036" > /sys/bus/i2c/drivers/max17040/unbind
> [   55.725123] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [   55.732584] pgd = d98d4000
> [   55.734060] [00000000] *pgd=5afa2831, *pte=00000000, *ppte=00000000
> [   55.740318] Internal error: Oops: 80000007 [#1] PREEMPT SMP ARM
> [   55.746210] Modules linked in:
> [   55.749259] CPU: 1 PID: 2936 Comm: cat Tainted: G        W       3.19.0-rc1-next-20141226-00048-gf79f475f3c44-dirty #1496
> [   55.760190] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [   55.766270] task: d9b76f00 ti: daf54000 task.ti: daf54000
> [   55.771647] PC is at 0x0
> [   55.774182] LR is at charger_get_property+0x2f4/0x36c
> [   55.779201] pc : [<00000000>]    lr : [<c034b0b4>]    psr: 60000013
> [   55.779201] sp : daf55e90  ip : 00000003  fp : 00000000
> [   55.790657] r10: 00000000  r9 : c06e2878  r8 : d9b26c68
> [   55.795865] r7 : dad81610  r6 : daec7410  r5 : daf55ebc  r4 : 00000000
> [   55.802367] r3 : 00000000  r2 : daf55ebc  r1 : 0000002a  r0 : d9b26c68
> [   55.808879] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> [   55.815994] Control: 10c5387d  Table: 598d406a  DAC: 00000015
> [   55.821723] Process cat (pid: 2936, stack limit = 0xdaf54210)
> [   55.827451] Stack: (0xdaf55e90 to 0xdaf56000)
> [   55.831795] 5e80:                                     60000013 c01459c4 0000002a c06f8ef8
> [   55.839956] 5ea0: db651000 c06f8ef8 daebac00 c04cb668 daebac08 c0346864 00000000 c01459c4
> [   55.848115] 5ec0: d99eaa80 c06f8ef8 00000fff 00001000 db651000 c027f25c c027f240 d99eaa80
> [   55.856274] 5ee0: d9a06c00 c0146218 daf55f18 00001000 d99eaa80 db4c18c0 00000001 00000001
> [   55.864468] 5f00: daf55f80 c0144c78 c0144c54 c0107f90 00015000 d99eaab0 00000000 00000000
> [   55.872603] 5f20: 000051c7 00000000 db4c18c0 c04a9370 00015000 00001000 daf55f80 00001000
> [   55.880763] 5f40: daf54000 00015000 00000000 c00e53dc db4c18c0 c00e548c 0000000d 00008124
> [   55.888937] 5f60: 00000001 00000000 00000000 db4c18c0 db4c18c0 00001000 00015000 c00e5550
> [   55.897099] 5f80: 00000000 00000000 00001000 00001000 00015000 00000003 00000003 c000f364
> [   55.905239] 5fa0: 00000000 c000f1a0 00001000 00015000 00000003 00015000 00001000 0001333c
> [   55.913399] 5fc0: 00001000 00015000 00000003 00000003 00000002 00000000 00000000 00000000
> [   55.921560] 5fe0: 7fffe000 be999850 0000a225 b6f3c19c 60000010 00000003 00000000 00000000
> [   55.929744] [<c034b0b4>] (charger_get_property) from [<c0346864>] (power_supply_show_property+0x48/0x20c)
> [   55.939286] [<c0346864>] (power_supply_show_property) from [<c027f25c>] (dev_attr_show+0x1c/0x48)
> [   55.948130] [<c027f25c>] (dev_attr_show) from [<c0146218>] (sysfs_kf_seq_show+0x84/0x104)
> [   55.956298] [<c0146218>] (sysfs_kf_seq_show) from [<c0144c78>] (kernfs_seq_show+0x24/0x28)
> [   55.964536] [<c0144c78>] (kernfs_seq_show) from [<c0107f90>] (seq_read+0x1b0/0x484)
> [   55.972172] [<c0107f90>] (seq_read) from [<c00e53dc>] (__vfs_read+0x18/0x4c)
> [   55.979188] [<c00e53dc>] (__vfs_read) from [<c00e548c>] (vfs_read+0x7c/0x100)
> [   55.986304] [<c00e548c>] (vfs_read) from [<c00e5550>] (SyS_read+0x40/0x8c)
> [   55.993164] [<c00e5550>] (SyS_read) from [<c000f1a0>] (ret_fast_syscall+0x0/0x48)
> [   56.000626] Code: bad PC value
> [   56.011652] ---[ end trace 7b64343fbdae8ef1 ]---
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> 
> [for the nvec part]
> Reviewed-by: Marc Dietrich <marvin24@gmx.de>
> 
> [for compal-laptop.c]
> Acked-by: Darren Hart <dvhart@linux.intel.com>
> 
> [for the mfd part]
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/x86/platform/olpc/olpc-xo1-sci.c     |   4 +-
>  arch/x86/platform/olpc/olpc-xo15-sci.c    |   4 +-
>  drivers/acpi/ac.c                         |  32 ++--
>  drivers/acpi/battery.c                    |  55 ++++---
>  drivers/acpi/sbs.c                        |  68 +++++----

Assuming that it has got enough testing,

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

for the ACPI changes above.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

  reply	other threads:[~2015-03-12 22:48 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-10  8:27 [PATCH v6 00/22] power_supply: Allow safe usage of power supply Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 01/22] compal-laptop: Fix leaking hwmon device Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 02/22] compal-laptop: Check return value of power_supply_register Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 03/22] power_supply: Add driver private data Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 04/22] power_supply: Move run-time configuration to separate structure Krzysztof Kozlowski
2015-03-11 16:06   ` Jiri Kosina
2015-03-10  8:27 ` [PATCH v6 05/22] power_supply: Add API for safe access of power supply function attrs Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 06/22] power_supply: sysfs: Use power_supply_*() API for accessing " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 07/22] power_supply: 88pm860x_charger: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 08/22] power_supply: ab8500: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 09/22] mfd: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 10/22] power_supply: apm_power: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 11/22] power_supply: bq2415x_charger: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 12/22] power_supply: charger-manager: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 13/22] power_supply: Change ownership from driver to core Krzysztof Kozlowski
2015-03-12 23:11   ` Rafael J. Wysocki [this message]
2015-03-10  8:27 ` [PATCH v6 14/22] power_supply: Add power_supply_put for decrementing device reference counter Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 15/22] power_supply: Increment power supply use counter when obtaining references Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 16/22] power_supply: charger-manager: Decrement the power supply's device reference counter Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 17/22] x86/olpc/xo1/sci: Use newly added power_supply_put API Krzysztof Kozlowski
2015-03-12 11:20   ` Ingo Molnar
2015-03-10  8:27 ` [PATCH v6 18/22] x86/olpc/xo15/sci: " Krzysztof Kozlowski
2015-03-12 11:21   ` Ingo Molnar
2015-03-10  8:27 ` [PATCH v6 19/22] power_supply: 88pm860x_charger: Decrement the power supply's device reference counter Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 20/22] power_supply: bq2415x_charger: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 21/22] mfd: ab8500: " Krzysztof Kozlowski
2015-03-10  8:27 ` [PATCH v6 22/22] arm: mach-pxa: " Krzysztof Kozlowski
2015-03-11  7:51 ` [PATCH v6 00/22] power_supply: Allow safe usage of power supply Krzysztof Kozlowski
2015-03-11  7:52 ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1926782.hW6KizxuR8@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=ac100@lists.launchpad.net \
    --cc=cezary.jackiewicz@gmail.com \
    --cc=daniel@zonque.org \
    --cc=dbaryshkov@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=dh.herrmann@googlemail.com \
    --cc=dvhart@infradead.org \
    --cc=dwmw2@infradead.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jkosina@suse.cz \
    --cc=k.kozlowski@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=lee.jones@linaro.org \
    --cc=lenb@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=milo.kim@ti.com \
    --cc=mingo@redhat.com \
    --cc=patches@opensource.wolfsonmicro.com \
    --cc=pavel@ucw.cz \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=robert.jarzmik@free.fr \
    --cc=sameo@linux.intel.com \
    --cc=sre@kernel.org \
    --cc=support.opensource@diasemi.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH v6 13/22] power_supply: Change ownership from driver to core' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).