Skip to content

Commit e5cb336

Browse files
krzkgregkh
authored andcommitted
power: charger-manager: Fix accessing invalidated power supply after charger unbind
commit cdaf3e1 upstream. The charger manager obtained in probe references to power supplies for all chargers with power_supply_get_by_name() for later usage. However if such charger driver was removed then this reference would point to old power supply (from driver which was removed). This lead to accessing invalid memory which could be observed with: $ echo "max77693-charger" > /sys/bus/platform/drivers/max77693-charger/unbind $ grep . /sys/devices/virtual/power_supply/battery/charger.0/* $ grep . /sys/devices/virtual/power_supply/battery/* [ 15.339817] Unable to handle kernel paging request at virtual address 0001c12c [ 15.346187] pgd = edd08000 [ 15.348814] [0001c12c] *pgd=6dce2831, *pte=00000000, *ppte=00000000 [ 15.355075] Internal error: Oops: 80000007 [#1] PREEMPT SMP ARM [ 15.360967] Modules linked in: [ 15.364010] CPU: 2 PID: 1388 Comm: grep Not tainted 3.17.0-next-20141007-00027-ga95e761db1b0 #245 [ 15.372859] task: ee03ad00 ti: edcf6000 task.ti: edcf6000 [ 15.378241] PC is at 0x1c12c [ 15.381113] LR is at is_ext_pwr_online+0x30/0x6c [ 15.385706] pc : [<0001c12c>] lr : [<c0339fc4>] psr: a0000013 [ 15.385706] sp : edcf7e88 ip : 00000000 fp : 00000000 [ 15.397161] r10: eeb02c08 r9 : c04b1f84 r8 : eeb02c00 [ 15.402369] r7 : edc69a10 r6 : eea6ac10 r5 : eea6ac10 r4 : 00000004 [ 15.408878] r3 : 0001c12c r2 : edcf7e8c r1 : 00000004 r0 : ee914418 [ 15.415390] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user [ 15.422506] Control: 10c5387d Table: 6dd0804a DAC: 00000015 [ 15.428236] Process grep (pid: 1388, stack limit = 0xedcf6240) [ 15.434050] Stack: (0xedcf7e88 to 0xedcf8000) [ 15.438395] 7e80: ee03ad00 00000000 edcf7f80 eea6aca8 edcf7ec4 c033b7b0 [ 15.446554] 7ea0: 00000001 ee1cc3f0 00000004 c06e1e44 eebdc000 c06e1e44 eeb02c00 c0337144 [ 15.454713] 7ec0: ee2dac68 c005cffc ee1cc3c0 c06e1e44 00000fff 00001000 eebdc000 c0278ca8 [ 15.462872] 7ee0: c0278c8c ee1cc3c0 eeb7ce00 c014422c edcf7f20 00008000 ee1cc3c0 ee9a48c0 [ 15.471030] 7f00: 00000001 00000001 edcf7f80 c0142d94 c0142d70 c01060f4 00021000 ee1cc3f0 [ 15.479190] 7f20: 00000000 00000000 c06a2150 eebdc000 2e7ec000 ee9a48c0 00008000 00021000 [ 15.487349] 7f40: edcf7f80 00008000 edcf6000 00021000 00021000 c00e39a4 00000000 ee9a48c0 [ 15.495508] 7f60: 00004000 00000000 00000000 ee9a48c0 ee9a48c0 00008000 00021000 c00e3aa0 [ 15.503668] 7f80: 00000000 00000000 0001f2e0 0001f2e0 00021000 00001000 00000003 c000f364 [ 15.511826] 7fa0: 00000000 c000f1a0 0001f2e0 00021000 00000003 00021000 00008000 00000000 [ 15.519986] 7fc0: 0001f2e0 00021000 00001000 00000003 00000001 000205e8 00000000 00021000 [ 15.528145] 7fe0: 00008000 bebbe910 0000a7ad b6edc49c 60000010 00000003 aaaaaaaa aaaaaaaa [ 15.536320] [<c0339fc4>] (is_ext_pwr_online) from [<c033b7b0>] (charger_get_property+0x170/0x314) [ 15.545164] [<c033b7b0>] (charger_get_property) from [<c0337144>] (power_supply_show_property+0x48/0x20c) [ 15.554719] [<c0337144>] (power_supply_show_property) from [<c0278ca8>] (dev_attr_show+0x1c/0x48) [ 15.563577] [<c0278ca8>] (dev_attr_show) from [<c014422c>] (sysfs_kf_seq_show+0x84/0x104) [ 15.571725] [<c014422c>] (sysfs_kf_seq_show) from [<c0142d94>] (kernfs_seq_show+0x24/0x28) [ 15.579973] [<c0142d94>] (kernfs_seq_show) from [<c01060f4>] (seq_read+0x1b0/0x484) [ 15.587614] [<c01060f4>] (seq_read) from [<c00e39a4>] (vfs_read+0x88/0x144) [ 15.594552] [<c00e39a4>] (vfs_read) from [<c00e3aa0>] (SyS_read+0x40/0x8c) [ 15.601417] [<c00e3aa0>] (SyS_read) from [<c000f1a0>] (ret_fast_syscall+0x0/0x48) [ 15.608877] Code: bad PC value [ 15.611991] ---[ end trace a88fcc95208db283 ]--- The charger-manager should get reference to charger power supply on each use of get_property callback. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Fixes: 3bb3dbb ("power_supply: Add initial Charger-Manager driver") Signed-off-by: Sebastian Reichel <sre@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 767a001 commit e5cb336

File tree

2 files changed

+39
-27
lines changed

2 files changed

+39
-27
lines changed

drivers/power/charger-manager.c

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,17 @@ static bool is_batt_present(struct charger_manager *cm)
118118
present = true;
119119
break;
120120
case CM_CHARGER_STAT:
121-
for (i = 0; cm->charger_stat[i]; i++) {
122-
ret = cm->charger_stat[i]->get_property(
123-
cm->charger_stat[i],
124-
POWER_SUPPLY_PROP_PRESENT, &val);
121+
for (i = 0; cm->desc->psy_charger_stat[i]; i++) {
122+
psy = power_supply_get_by_name(
123+
cm->desc->psy_charger_stat[i]);
124+
if (!psy) {
125+
dev_err(cm->dev, "Cannot find power supply \"%s\"\n",
126+
cm->desc->psy_charger_stat[i]);
127+
continue;
128+
}
129+
130+
ret = psy->get_property(psy, POWER_SUPPLY_PROP_PRESENT,
131+
&val);
125132
if (ret == 0 && val.intval) {
126133
present = true;
127134
break;
@@ -144,14 +151,20 @@ static bool is_batt_present(struct charger_manager *cm)
144151
static bool is_ext_pwr_online(struct charger_manager *cm)
145152
{
146153
union power_supply_propval val;
154+
struct power_supply *psy;
147155
bool online = false;
148156
int i, ret;
149157

150158
/* If at least one of them has one, it's yes. */
151-
for (i = 0; cm->charger_stat[i]; i++) {
152-
ret = cm->charger_stat[i]->get_property(
153-
cm->charger_stat[i],
154-
POWER_SUPPLY_PROP_ONLINE, &val);
159+
for (i = 0; cm->desc->psy_charger_stat[i]; i++) {
160+
psy = power_supply_get_by_name(cm->desc->psy_charger_stat[i]);
161+
if (!psy) {
162+
dev_err(cm->dev, "Cannot find power supply \"%s\"\n",
163+
cm->desc->psy_charger_stat[i]);
164+
continue;
165+
}
166+
167+
ret = psy->get_property(psy, POWER_SUPPLY_PROP_ONLINE, &val);
155168
if (ret == 0 && val.intval) {
156169
online = true;
157170
break;
@@ -196,24 +209,30 @@ static bool is_charging(struct charger_manager *cm)
196209
{
197210
int i, ret;
198211
bool charging = false;
212+
struct power_supply *psy;
199213
union power_supply_propval val;
200214

201215
/* If there is no battery, it cannot be charged */
202216
if (!is_batt_present(cm))
203217
return false;
204218

205219
/* If at least one of the charger is charging, return yes */
206-
for (i = 0; cm->charger_stat[i]; i++) {
220+
for (i = 0; cm->desc->psy_charger_stat[i]; i++) {
207221
/* 1. The charger sholuld not be DISABLED */
208222
if (cm->emergency_stop)
209223
continue;
210224
if (!cm->charger_enabled)
211225
continue;
212226

227+
psy = power_supply_get_by_name(cm->desc->psy_charger_stat[i]);
228+
if (!psy) {
229+
dev_err(cm->dev, "Cannot find power supply \"%s\"\n",
230+
cm->desc->psy_charger_stat[i]);
231+
continue;
232+
}
233+
213234
/* 2. The charger should be online (ext-power) */
214-
ret = cm->charger_stat[i]->get_property(
215-
cm->charger_stat[i],
216-
POWER_SUPPLY_PROP_ONLINE, &val);
235+
ret = psy->get_property(psy, POWER_SUPPLY_PROP_ONLINE, &val);
217236
if (ret) {
218237
dev_warn(cm->dev, "Cannot read ONLINE value from %s\n",
219238
cm->desc->psy_charger_stat[i]);
@@ -226,9 +245,7 @@ static bool is_charging(struct charger_manager *cm)
226245
* 3. The charger should not be FULL, DISCHARGING,
227246
* or NOT_CHARGING.
228247
*/
229-
ret = cm->charger_stat[i]->get_property(
230-
cm->charger_stat[i],
231-
POWER_SUPPLY_PROP_STATUS, &val);
248+
ret = psy->get_property(psy, POWER_SUPPLY_PROP_STATUS, &val);
232249
if (ret) {
233250
dev_warn(cm->dev, "Cannot read STATUS value from %s\n",
234251
cm->desc->psy_charger_stat[i]);
@@ -1772,15 +1789,12 @@ static int charger_manager_probe(struct platform_device *pdev)
17721789
while (desc->psy_charger_stat[i])
17731790
i++;
17741791

1775-
cm->charger_stat = devm_kzalloc(&pdev->dev,
1776-
sizeof(struct power_supply *) * i, GFP_KERNEL);
1777-
if (!cm->charger_stat)
1778-
return -ENOMEM;
1779-
1792+
/* Check if charger's supplies are present at probe */
17801793
for (i = 0; desc->psy_charger_stat[i]; i++) {
1781-
cm->charger_stat[i] = power_supply_get_by_name(
1782-
desc->psy_charger_stat[i]);
1783-
if (!cm->charger_stat[i]) {
1794+
struct power_supply *psy;
1795+
1796+
psy = power_supply_get_by_name(desc->psy_charger_stat[i]);
1797+
if (!psy) {
17841798
dev_err(&pdev->dev, "Cannot find power supply \"%s\"\n",
17851799
desc->psy_charger_stat[i]);
17861800
return -ENODEV;
@@ -2102,8 +2116,8 @@ static bool find_power_supply(struct charger_manager *cm,
21022116
int i;
21032117
bool found = false;
21042118

2105-
for (i = 0; cm->charger_stat[i]; i++) {
2106-
if (psy == cm->charger_stat[i]) {
2119+
for (i = 0; cm->desc->psy_charger_stat[i]; i++) {
2120+
if (!strcmp(psy->name, cm->desc->psy_charger_stat[i])) {
21072121
found = true;
21082122
break;
21092123
}

include/linux/power/charger-manager.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,6 @@ struct charger_manager {
253253
struct device *dev;
254254
struct charger_desc *desc;
255255

256-
struct power_supply **charger_stat;
257-
258256
#ifdef CONFIG_THERMAL
259257
struct thermal_zone_device *tzd_batt;
260258
#endif

0 commit comments

Comments
 (0)