linux-kernel.vger.kernel.org archive mirror
 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>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: "H. Nikolaus Schaller" <hns@goldelico.com>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>
Subject: [RFC/RFT 1/2] power_supply: Fix NULL pointer dereference during bq27x00_battery probe
Date: Tue, 19 May 2015 14:26:02 +0900	[thread overview]
Message-ID: <1432013163-20713-2-git-send-email-k.kozlowski@samsung.com> (raw)
In-Reply-To: <1432013163-20713-1-git-send-email-k.kozlowski@samsung.com>

Power supply is often registered during probe of a driver. The
power_supply_register() returns pointer to newly allocated structure as
return value. However before returning the power_supply_register()
calls back the get_property() method provided by the driver through
uevent.

In that time the driver probe is still in progress and driver did not
assigned pointer to power supply to its local variables. This leads to
NULL pointer dereference from get_property() function.
Starting from bq27x00_battery_probe():
  di->bat = power_supply_register()
    device_add()
      kobject_uevent()
        power_supply_uevent()
          power_supply_show_property()
            power_supply_get_property()
              bq27x00_battery_get_property()
                dereference of (di->bat) which is NULL here

The first uevent of power supply (the one coming from device creation)
should not call back to the driver. To prevent that from happening,
increment the atomic use counter at the end of power_supply_register().
This means that power_supply_get_property() will return -ENODEV.

IMPORTANT:
The patch has impact on this first uevent sent from power supply because
it will not contain properties from power supply.

The uevent with properties will be sent later after indicating that
power supply has changed. This also has a race now, but will be fixed in
other patches.

Reported-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Fixes: 297d716f6260 ("power_supply: Change ownership from driver to core")
---
 drivers/power/power_supply_core.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 2ed4a4a6b3c5..15da277e0e8d 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -659,7 +659,6 @@ __power_supply_register(struct device *parent,
 	dev->release = power_supply_dev_release;
 	dev_set_drvdata(dev, psy);
 	psy->desc = desc;
-	atomic_inc(&psy->use_cnt);
 	if (cfg) {
 		psy->drv_data = cfg->drv_data;
 		psy->of_node = cfg->of_node;
@@ -700,6 +699,16 @@ __power_supply_register(struct device *parent,
 	if (rc)
 		goto create_triggers_failed;
 
+	/*
+	 * Update use_cnt after any uevents (most notably from device_add()).
+	 * We are here still during driver's probe but
+	 * the power_supply_uevent() calls back driver's get_property
+	 * method so:
+	 * 1. Driver did not assigned the returned struct power_supply,
+	 * 2. Driver could not finish initialization (anything in its probe
+	 *    after calling power_supply_register()).
+	 */
+	atomic_inc(&psy->use_cnt);
 	power_supply_changed(psy);
 
 	return psy;
-- 
1.9.1


  reply	other threads:[~2015-05-19  5:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-19  5:26 [RFC/RFT 0/2] power_supply: Fix NULL pointer dereference from uevent Krzysztof Kozlowski
2015-05-19  5:26 ` Krzysztof Kozlowski [this message]
2015-05-19  5:26 ` [RFC/RFT 2/2] power_supply: Fix possible NULL pointer dereference on early uevent Krzysztof Kozlowski
2015-05-19  6:10   ` 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=1432013163-20713-2-git-send-email-k.kozlowski@samsung.com \
    --to=k.kozlowski@samsung.com \
    --cc=dbaryshkov@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=hns@goldelico.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=sre@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).