LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Sebastian Reichel <sre@kernel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	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
Cc: Thomas Gleixner <tglx@linutronix.de>, Pavel Machek <pavel@ucw.cz>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>
Subject: [PATCH v6 00/22] power_supply: Allow safe usage of power supply
Date: Tue, 10 Mar 2015 09:27:04 +0100	[thread overview]
Message-ID: <1425976046-20973-1-git-send-email-k.kozlowski@samsung.com> (raw)

Hi,


The patchset fixes invalid memory accesses in certain race scenarios by
moving ownership of struct power_supply to the core. All drivers are
modified.


TLDR for driver and subsystem maintainers
=========================================
Two patches of patchset change power_supply_register() function so in
the same time they touch all drivers. I am kindly asking for acks,
review and help in testing.

Especially please take a look at:
 - patch 4: power_supply: Move run-time configuration to separate structure
 - patch 13: power_supply: Change ownership from driver to core

I still need acks for changes in subsystems:
1. x86/olpc,
2. ACPI,
3. HID.


Problem to fix
==============
Patchset tries to fix invalid memory accesses occurring in following
race scenario:

Thread 1: charger manager, CONSUMER
Thread 2: power supply driver, PROVIDER

THREAD 1 (consumer)                THREAD 2 (provider)
===================                ===================
psy = power_supply_get_by_name()
                                   Driver unbind, .remove
                                     power_supply_unregister()
                                     Device fully removed
psy->get_property()

The consumer of power supply will reference invalid memory because
the provider already freed it.


The fix
=======
To properly fix the race the patchset:
1. Moves ownership of power_supply structure from driver (provider) to
   power supply core.
2. Adds power_supply_get_property()-like API for safe access by consumer.
3. Adds power_supply_put() which will reclaim memory.


I modifed all drivers I found. However I only compile tested them
(plus Smatch, Sparse and coccicheck). I did not test them on real
hardware (except max14577, max77693, max17040, max17042
and charger-manager).


What the patchset does in steps
===============================
1. Some preparation steps are necessary - patch 3 and 4. The driver
   implementing power supply won't be able to fill structure before
   calling power_supply_register(). So 'power_supply_config'
   is introduced in patch 4 ("power_supply: Move run-time configuration
   to separate structure"). Unfortunately this touches all drivers.
   *All drivers are touched.*

2. Safe API wrappers (and usage counter) are added (power_supply_*()).

3. Patch 13: ownership of 'struct power_supply' is moved from driver
   to the core.
   *All drivers are touched.*

4. power_supply_put() is added which reclaims resources.


Dependencies
============
The patchset is rebased on v4.0-rc1 and latest battery tree.

Bisect-ability is preserved. All later patches depend on previous ones
so it could be pulled in steps, but cherry-picking won't work.

Patchset is available also here:
https://git.linaro.org/people/marek.szyprowski/linux-srpol.git
branch: v4.0-rc1-power-supply-core-ownership


Changes since v5
================
1. Add two fixes for compal-laptop to beginning of this patchset
   to avoid conflicts. They touch the same place as this patchset.
   I got acks for them (Darren Hart).
2. Rebased on current battery tree. This introduced new changes:
   - updated bq27x00_battery driver,
   - new driver: axp288_fuel_gauge.
3. Add acks: Pavel Machek's (patch 22/22), Darren Hart (p. 1 and 2),
   Guenter Roeck (p. 1).

Changes since v4
================
1. Rebased on current battery tree. This introduced new changes:
   - new devm-like register functions,
   - new driver: da9150-charger.
2. Add acks: Darren Hart (compal-laptop.c, p. 11), Lee Jones
   (mfd, p. 11).

Changes since v3
================
1. Fix Jonghwa's email in his ack (spotted by Stefan Wahren)
2. Update power_supply_register() documentation.
3. Fix putting the reference to power supply in bq2415x_charger.c (put
   it when it is non-NULL).
4. Add acks: Robert Jarzmik (patch 20), Darren Hart (p. 2), Pavel
   Machek (p. 1-3), Marc Dietrich (p. 2 and 11).

Changes since v2
================
1. Rewrite all drivers to new power_supply_register().
2. Add reviewed-by Bartlomiej Zolnierkiewicz (internal review)
3. Add reviewed-by Sebastian Reichel [2] (to patches which I did not
   change in major way between v2 and v3).
4. Use atomic usage counter of power supply on each of:
   a. register/unregister,
   b. get/put.

Changes since v1
================
1. Add new patches (1, 2, 11, 19).
2. Preserved ack-s where there weren't any changes.
3. Patch 3: Add use counter.
4. Patch 3: Don't add wrapper for set_charged() because already exists
   one.


[1] https://lkml.org/lkml/2015/1/21/471
[2] https://lkml.org/lkml/2015/2/20/150

Best regards,
Krzysztof


Krzysztof Kozlowski (22):
  compal-laptop: Fix leaking hwmon device
  compal-laptop: Check return value of power_supply_register
  power_supply: Add driver private data
  power_supply: Move run-time configuration to separate structure
  power_supply: Add API for safe access of power supply function attrs
  power_supply: sysfs: Use power_supply_*() API for accessing function
    attrs
  power_supply: 88pm860x_charger: Use power_supply_*() API for accessing
    function attrs
  power_supply: ab8500: Use power_supply_*() API for accessing function
    attrs
  mfd: ab8500: Use power_supply_*() API for accessing function attrs
  power_supply: apm_power: Use power_supply_*() API for accessing
    function attrs
  power_supply: bq2415x_charger: Use power_supply_*() API for accessing
    function attrs
  power_supply: charger-manager: Use power_supply_*() API for accessing
    function attrs
  power_supply: Change ownership from driver to core
  power_supply: Add power_supply_put for decrementing device reference
    counter
  power_supply: Increment power supply use counter when obtaining
    references
  power_supply: charger-manager: Decrement the power supply's device
    reference counter
  x86/olpc/xo1/sci: Use newly added power_supply_put API
  x86/olpc/xo15/sci: Use newly added power_supply_put API
  power_supply: 88pm860x_charger: Decrement the power supply's device
    reference counter
  power_supply: bq2415x_charger: Decrement the power supply's device
    reference counter
  mfd: ab8500: Decrement the power supply's device reference counter
  arm: mach-pxa: Decrement the power supply's device reference counter

 arch/arm/mach-pxa/raumfeld.c              |   4 +-
 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                    |  54 ++---
 drivers/acpi/sbs.c                        |  68 +++---
 drivers/hid/hid-input.c                   |  51 +++--
 drivers/hid/hid-sony.c                    |  43 ++--
 drivers/hid/hid-wiimote-modules.c         |  41 ++--
 drivers/hid/hid-wiimote.h                 |   3 +-
 drivers/hid/wacom.h                       |   8 +-
 drivers/hid/wacom_sys.c                   |  70 +++---
 drivers/mfd/ab8500-sysctrl.c              |   9 +-
 drivers/platform/x86/compal-laptop.c      |  35 +--
 drivers/power/88pm860x_battery.c          |  40 ++--
 drivers/power/88pm860x_charger.c          |  61 ++++--
 drivers/power/ab8500_btemp.c              |  75 ++++---
 drivers/power/ab8500_charger.c            | 139 ++++++------
 drivers/power/ab8500_fg.c                 | 130 +++++------
 drivers/power/abx500_chargalg.c           |  98 ++++-----
 drivers/power/apm_power.c                 |   6 +-
 drivers/power/axp288_fuel_gauge.c         |  47 ++--
 drivers/power/bq2415x_charger.c           | 107 +++++-----
 drivers/power/bq24190_charger.c           | 103 +++++----
 drivers/power/bq24735-charger.c           |  53 ++---
 drivers/power/bq27x00_battery.c           |  74 +++----
 drivers/power/charger-manager.c           | 159 ++++++++------
 drivers/power/collie_battery.c            |  75 ++++---
 drivers/power/da9030_battery.c            |  33 +--
 drivers/power/da9052-battery.c            |  25 ++-
 drivers/power/da9150-charger.c            |  80 +++----
 drivers/power/ds2760_battery.c            |  56 ++---
 drivers/power/ds2780_battery.c            |  45 ++--
 drivers/power/ds2781_battery.c            |  47 ++--
 drivers/power/ds2782_battery.c            |  30 +--
 drivers/power/generic-adc-battery.c       |  54 ++---
 drivers/power/goldfish_battery.c          |  63 +++---
 drivers/power/gpio-charger.c              |  42 ++--
 drivers/power/intel_mid_battery.c         |  57 +++--
 drivers/power/ipaq_micro_battery.c        |  34 +--
 drivers/power/isp1704_charger.c           |  49 +++--
 drivers/power/jz4740-battery.c            |  37 ++--
 drivers/power/lp8727_charger.c            |  94 ++++----
 drivers/power/lp8788-charger.c            |  62 +++---
 drivers/power/ltc2941-battery-gauge.c     |  51 +++--
 drivers/power/max14577_charger.c          |  34 +--
 drivers/power/max17040_battery.c          |  31 +--
 drivers/power/max17042_battery.c          |  45 ++--
 drivers/power/max77693_charger.c          |  32 +--
 drivers/power/max8903_charger.c           |  52 ++---
 drivers/power/max8925_power.c             |  98 +++++----
 drivers/power/max8997_charger.c           |  31 +--
 drivers/power/max8998_charger.c           |  32 +--
 drivers/power/olpc_battery.c              |  54 ++---
 drivers/power/pcf50633-charger.c          | 105 +++++----
 drivers/power/pda_power.c                 |  66 +++---
 drivers/power/pm2301_charger.c            |  48 +++--
 drivers/power/pm2301_charger.h            |   1 +
 drivers/power/pmu_battery.c               |  42 ++--
 drivers/power/power_supply_core.c         | 344 +++++++++++++++++++++++-------
 drivers/power/power_supply_leds.c         |  25 +--
 drivers/power/power_supply_sysfs.c        |  24 +--
 drivers/power/rt5033_battery.c            |  27 ++-
 drivers/power/rx51_battery.c              |  27 ++-
 drivers/power/s3c_adc_battery.c           |  77 ++++---
 drivers/power/sbs-battery.c               |  71 +++---
 drivers/power/smb347-charger.c            | 108 +++++-----
 drivers/power/test_power.c                |  53 +++--
 drivers/power/tosa_battery.c              | 112 ++++++----
 drivers/power/tps65090-charger.c          |  43 ++--
 drivers/power/twl4030_charger.c           |  65 +++---
 drivers/power/twl4030_madc_battery.c      |  41 ++--
 drivers/power/wm831x_backup.c             |  26 ++-
 drivers/power/wm831x_power.c              |  95 +++++----
 drivers/power/wm8350_power.c              |  89 ++++----
 drivers/power/wm97xx_battery.c            |  37 ++--
 drivers/power/z2_battery.c                |  60 +++---
 drivers/staging/nvec/nvec_power.c         |  34 +--
 include/linux/hid.h                       |   6 +-
 include/linux/mfd/abx500/ux500_chargalg.h |  11 +-
 include/linux/mfd/rt5033.h                |   2 +-
 include/linux/mfd/wm8350/supply.h         |   6 +-
 include/linux/power/charger-manager.h     |   3 +-
 include/linux/power_supply.h              |  82 +++++--
 84 files changed, 2639 insertions(+), 1952 deletions(-)

-- 
1.9.1


             reply	other threads:[~2015-03-10  8:27 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-10  8:27 Krzysztof Kozlowski [this message]
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
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=1425976046-20973-1-git-send-email-k.kozlowski@samsung.com \
    --to=k.kozlowski@samsung.com \
    --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=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=rjw@rjwysocki.net \
    --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 00/22] power_supply: Allow safe usage of power supply' \
    /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).