Skip to content
This repository was archived by the owner on Dec 20, 2023. It is now read-only.

Commit bdbe814

Browse files
krzksre
krzk
authored and
sre
committed
power: charger-manager: Fix accessing invalidated power supply after fuel gauge unbind
The charger manager obtained reference to fuel gauge power supply in probe with power_supply_get_by_name() for later usage. However if fuel gauge driver was removed and re-added then this reference would point to old power supply (from driver which was removed). This lead to accessing old (and probably invalid) memory which could be observed with: $ echo "12-0036" > /sys/bus/i2c/drivers/max17042/unbind $ echo "12-0036" > /sys/bus/i2c/drivers/max17042/bind $ cat /sys/devices/virtual/power_supply/battery/capacity [ 240.480084] INFO: task cat:1393 blocked for more than 120 seconds. [ 240.484799] Not tainted 3.17.0-next-20141007-00028-ge60b6dd79570 torvalds#203 [ 240.491782] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 240.499589] cat D c0469530 0 1393 1 0x00000000 [ 240.505947] [<c0469530>] (__schedule) from [<c0469d3c>] (schedule_preempt_disabled+0x14/0x20) [ 240.514449] [<c0469d3c>] (schedule_preempt_disabled) from [<c046af08>] (mutex_lock_nested+0x1bc/0x458) [ 240.523736] [<c046af08>] (mutex_lock_nested) from [<c0287a98>] (regmap_read+0x30/0x60) [ 240.531647] [<c0287a98>] (regmap_read) from [<c032238c>] (max17042_get_property+0x2e8/0x350) [ 240.540055] [<c032238c>] (max17042_get_property) from [<c03247d8>] (charger_get_property+0x264/0x348) [ 240.549252] [<c03247d8>] (charger_get_property) from [<c0320764>] (power_supply_show_property+0x48/0x1e0) [ 240.558808] [<c0320764>] (power_supply_show_property) from [<c027308c>] (dev_attr_show+0x1c/0x48) [ 240.567664] [<c027308c>] (dev_attr_show) from [<c0141fb0>] (sysfs_kf_seq_show+0x84/0x104) [ 240.575814] [<c0141fb0>] (sysfs_kf_seq_show) from [<c0140b18>] (kernfs_seq_show+0x24/0x28) [ 240.584061] [<c0140b18>] (kernfs_seq_show) from [<c0104574>] (seq_read+0x1b0/0x484) [ 240.591702] [<c0104574>] (seq_read) from [<c00e1e24>] (vfs_read+0x88/0x144) [ 240.598640] [<c00e1e24>] (vfs_read) from [<c00e1f20>] (SyS_read+0x40/0x8c) [ 240.605507] [<c00e1f20>] (SyS_read) from [<c000e760>] (ret_fast_syscall+0x0/0x48) [ 240.612952] 4 locks held by cat/1393: [ 240.616589] #0: (&p->lock){+.+.+.}, at: [<c01043f4>] seq_read+0x30/0x484 [ 240.623414] #1: (&of->mutex){+.+.+.}, at: [<c01417dc>] kernfs_seq_start+0x1c/0x8c [ 240.631086] #2: (s_active#31){++++.+}, at: [<c01417e4>] kernfs_seq_start+0x24/0x8c [ 240.638777] #3: (&map->mutex){+.+...}, at: [<c0287a98>] regmap_read+0x30/0x60 The charger-manager should get reference to fuel gauge power supply on each use of get_property callback. The thermal zone 'tzd' field of power supply should not be used because of the same reason. Additionally this change solves also the issue with nested thermal_zone_get_temp() calls and related false lockdep positive for deadlock for thermal zone's mutex [1]. When fuel gauge is used as source of temperature then the charger manager forwards its get_temp calls to fuel gauge thermal zone. So actually different mutexes are used (one for charger manager thermal zone and second for fuel gauge thermal zone) but for lockdep this is one class of mutex. The recursion is removed by retrieving temperature through power supply's get_property(). In case external thermal zone is used ('cm-thermal-zone' property is present in DTS) the recursion does not exist. Charger manager simply exports POWER_SUPPLY_PROP_TEMP_AMBIENT property (instead of POWER_SUPPLY_PROP_TEMP) thus no thermal zone is created for this power supply. [1] https://lkml.org/lkml/2014/10/6/309 Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Cc: <stable@vger.kernel.org> Fixes: 3bb3dbb ("power_supply: Add initial Charger-Manager driver") Signed-off-by: Sebastian Reichel <sre@kernel.org>
1 parent ba9c918 commit bdbe814

File tree

2 files changed

+71
-29
lines changed

2 files changed

+71
-29
lines changed

‎drivers/power/charger-manager.c

+71-28
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ static struct charger_global_desc *g_desc; /* init with setup_charger_manager */
9797
static bool is_batt_present(struct charger_manager *cm)
9898
{
9999
union power_supply_propval val;
100+
struct power_supply *psy;
100101
bool present = false;
101102
int i, ret;
102103

@@ -107,7 +108,11 @@ static bool is_batt_present(struct charger_manager *cm)
107108
case CM_NO_BATTERY:
108109
break;
109110
case CM_FUEL_GAUGE:
110-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
111+
psy = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
112+
if (!psy)
113+
break;
114+
115+
ret = psy->get_property(psy,
111116
POWER_SUPPLY_PROP_PRESENT, &val);
112117
if (ret == 0 && val.intval)
113118
present = true;
@@ -167,12 +172,14 @@ static bool is_ext_pwr_online(struct charger_manager *cm)
167172
static int get_batt_uV(struct charger_manager *cm, int *uV)
168173
{
169174
union power_supply_propval val;
175+
struct power_supply *fuel_gauge;
170176
int ret;
171177

172-
if (!cm->fuel_gauge)
178+
fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
179+
if (!fuel_gauge)
173180
return -ENODEV;
174181

175-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
182+
ret = fuel_gauge->get_property(fuel_gauge,
176183
POWER_SUPPLY_PROP_VOLTAGE_NOW, &val);
177184
if (ret)
178185
return ret;
@@ -248,18 +255,23 @@ static bool is_full_charged(struct charger_manager *cm)
248255
{
249256
struct charger_desc *desc = cm->desc;
250257
union power_supply_propval val;
258+
struct power_supply *fuel_gauge;
251259
int ret = 0;
252260
int uV;
253261

254262
/* If there is no battery, it cannot be charged */
255263
if (!is_batt_present(cm))
256264
return false;
257265

258-
if (cm->fuel_gauge && desc->fullbatt_full_capacity > 0) {
266+
fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
267+
if (!fuel_gauge)
268+
return false;
269+
270+
if (desc->fullbatt_full_capacity > 0) {
259271
val.intval = 0;
260272

261273
/* Not full if capacity of fuel gauge isn't full */
262-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
274+
ret = fuel_gauge->get_property(fuel_gauge,
263275
POWER_SUPPLY_PROP_CHARGE_FULL, &val);
264276
if (!ret && val.intval > desc->fullbatt_full_capacity)
265277
return true;
@@ -273,10 +285,10 @@ static bool is_full_charged(struct charger_manager *cm)
273285
}
274286

275287
/* Full, if the capacity is more than fullbatt_soc */
276-
if (cm->fuel_gauge && desc->fullbatt_soc > 0) {
288+
if (desc->fullbatt_soc > 0) {
277289
val.intval = 0;
278290

279-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
291+
ret = fuel_gauge->get_property(fuel_gauge,
280292
POWER_SUPPLY_PROP_CAPACITY, &val);
281293
if (!ret && val.intval >= desc->fullbatt_soc)
282294
return true;
@@ -551,6 +563,20 @@ static int check_charging_duration(struct charger_manager *cm)
551563
return ret;
552564
}
553565

566+
static int cm_get_battery_temperature_by_psy(struct charger_manager *cm,
567+
int *temp)
568+
{
569+
struct power_supply *fuel_gauge;
570+
571+
fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
572+
if (!fuel_gauge)
573+
return -ENODEV;
574+
575+
return fuel_gauge->get_property(fuel_gauge,
576+
POWER_SUPPLY_PROP_TEMP,
577+
(union power_supply_propval *)temp);
578+
}
579+
554580
static int cm_get_battery_temperature(struct charger_manager *cm,
555581
int *temp)
556582
{
@@ -560,15 +586,18 @@ static int cm_get_battery_temperature(struct charger_manager *cm,
560586
return -ENODEV;
561587

562588
#ifdef CONFIG_THERMAL
563-
ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp);
564-
if (!ret)
565-
/* Calibrate temperature unit */
566-
*temp /= 100;
567-
#else
568-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
569-
POWER_SUPPLY_PROP_TEMP,
570-
(union power_supply_propval *)temp);
589+
if (cm->tzd_batt) {
590+
ret = thermal_zone_get_temp(cm->tzd_batt, (unsigned long *)temp);
591+
if (!ret)
592+
/* Calibrate temperature unit */
593+
*temp /= 100;
594+
} else
571595
#endif
596+
{
597+
/* if-else continued from CONFIG_THERMAL */
598+
ret = cm_get_battery_temperature_by_psy(cm, temp);
599+
}
600+
572601
return ret;
573602
}
574603

@@ -827,6 +856,7 @@ static int charger_get_property(struct power_supply *psy,
827856
struct charger_manager *cm = container_of(psy,
828857
struct charger_manager, charger_psy);
829858
struct charger_desc *desc = cm->desc;
859+
struct power_supply *fuel_gauge;
830860
int ret = 0;
831861
int uV;
832862

@@ -857,14 +887,20 @@ static int charger_get_property(struct power_supply *psy,
857887
ret = get_batt_uV(cm, &val->intval);
858888
break;
859889
case POWER_SUPPLY_PROP_CURRENT_NOW:
860-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
890+
fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
891+
if (!fuel_gauge) {
892+
ret = -ENODEV;
893+
break;
894+
}
895+
ret = fuel_gauge->get_property(fuel_gauge,
861896
POWER_SUPPLY_PROP_CURRENT_NOW, val);
862897
break;
863898
case POWER_SUPPLY_PROP_TEMP:
864899
case POWER_SUPPLY_PROP_TEMP_AMBIENT:
865900
return cm_get_battery_temperature(cm, &val->intval);
866901
case POWER_SUPPLY_PROP_CAPACITY:
867-
if (!cm->fuel_gauge) {
902+
fuel_gauge = power_supply_get_by_name(cm->desc->psy_fuel_gauge);
903+
if (!fuel_gauge) {
868904
ret = -ENODEV;
869905
break;
870906
}
@@ -875,7 +911,7 @@ static int charger_get_property(struct power_supply *psy,
875911
break;
876912
}
877913

878-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
914+
ret = fuel_gauge->get_property(fuel_gauge,
879915
POWER_SUPPLY_PROP_CAPACITY, val);
880916
if (ret)
881917
break;
@@ -924,7 +960,14 @@ static int charger_get_property(struct power_supply *psy,
924960
break;
925961
case POWER_SUPPLY_PROP_CHARGE_NOW:
926962
if (is_charging(cm)) {
927-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
963+
fuel_gauge = power_supply_get_by_name(
964+
cm->desc->psy_fuel_gauge);
965+
if (!fuel_gauge) {
966+
ret = -ENODEV;
967+
break;
968+
}
969+
970+
ret = fuel_gauge->get_property(fuel_gauge,
928971
POWER_SUPPLY_PROP_CHARGE_NOW,
929972
val);
930973
if (ret) {
@@ -1486,14 +1529,15 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
14861529
return ret;
14871530
}
14881531

1489-
static int cm_init_thermal_data(struct charger_manager *cm)
1532+
static int cm_init_thermal_data(struct charger_manager *cm,
1533+
struct power_supply *fuel_gauge)
14901534
{
14911535
struct charger_desc *desc = cm->desc;
14921536
union power_supply_propval val;
14931537
int ret;
14941538

14951539
/* Verify whether fuel gauge provides battery temperature */
1496-
ret = cm->fuel_gauge->get_property(cm->fuel_gauge,
1540+
ret = fuel_gauge->get_property(fuel_gauge,
14971541
POWER_SUPPLY_PROP_TEMP, &val);
14981542

14991543
if (!ret) {
@@ -1503,8 +1547,6 @@ static int cm_init_thermal_data(struct charger_manager *cm)
15031547
cm->desc->measure_battery_temp = true;
15041548
}
15051549
#ifdef CONFIG_THERMAL
1506-
cm->tzd_batt = cm->fuel_gauge->tzd;
1507-
15081550
if (ret && desc->thermal_zone) {
15091551
cm->tzd_batt =
15101552
thermal_zone_get_zone_by_name(desc->thermal_zone);
@@ -1667,6 +1709,7 @@ static int charger_manager_probe(struct platform_device *pdev)
16671709
int ret = 0, i = 0;
16681710
int j = 0;
16691711
union power_supply_propval val;
1712+
struct power_supply *fuel_gauge;
16701713

16711714
if (g_desc && !rtc_dev && g_desc->rtc_name) {
16721715
rtc_dev = rtc_class_open(g_desc->rtc_name);
@@ -1745,8 +1788,8 @@ static int charger_manager_probe(struct platform_device *pdev)
17451788
}
17461789
}
17471790

1748-
cm->fuel_gauge = power_supply_get_by_name(desc->psy_fuel_gauge);
1749-
if (!cm->fuel_gauge) {
1791+
fuel_gauge = power_supply_get_by_name(desc->psy_fuel_gauge);
1792+
if (!fuel_gauge) {
17501793
dev_err(&pdev->dev, "Cannot find power supply \"%s\"\n",
17511794
desc->psy_fuel_gauge);
17521795
return -ENODEV;
@@ -1789,21 +1832,21 @@ static int charger_manager_probe(struct platform_device *pdev)
17891832
cm->charger_psy.num_properties = psy_default.num_properties;
17901833

17911834
/* Find which optional psy-properties are available */
1792-
if (!cm->fuel_gauge->get_property(cm->fuel_gauge,
1835+
if (!fuel_gauge->get_property(fuel_gauge,
17931836
POWER_SUPPLY_PROP_CHARGE_NOW, &val)) {
17941837
cm->charger_psy.properties[cm->charger_psy.num_properties] =
17951838
POWER_SUPPLY_PROP_CHARGE_NOW;
17961839
cm->charger_psy.num_properties++;
17971840
}
1798-
if (!cm->fuel_gauge->get_property(cm->fuel_gauge,
1841+
if (!fuel_gauge->get_property(fuel_gauge,
17991842
POWER_SUPPLY_PROP_CURRENT_NOW,
18001843
&val)) {
18011844
cm->charger_psy.properties[cm->charger_psy.num_properties] =
18021845
POWER_SUPPLY_PROP_CURRENT_NOW;
18031846
cm->charger_psy.num_properties++;
18041847
}
18051848

1806-
ret = cm_init_thermal_data(cm);
1849+
ret = cm_init_thermal_data(cm, fuel_gauge);
18071850
if (ret) {
18081851
dev_err(&pdev->dev, "Failed to initialize thermal data\n");
18091852
cm->desc->measure_battery_temp = false;

‎include/linux/power/charger-manager.h

-1
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,6 @@ struct charger_manager {
253253
struct device *dev;
254254
struct charger_desc *desc;
255255

256-
struct power_supply *fuel_gauge;
257256
struct power_supply **charger_stat;
258257

259258
#ifdef CONFIG_THERMAL

0 commit comments

Comments
 (0)