From 4c710643e550961b5a86415dab2b0bdaab5c8081 Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Wed, 24 Aug 2022 12:03:53 -0700 Subject: [PATCH 1/2] power: supply: qti_battery_charger: Fix unintended invocation of state_cb() Currently, pmic_glink_register_client() is getting called even before battery_chg_register_panel_notifier() is called. However, if of_drm_find_panel() doesn't return an active panel, then battery_chg_register_panel_notifier() returns -EPROBE_DEFER which would end up in calling pmic_glink_unregister_client() and this would happen forever. Though this error return path is handled properly, when SSR/PDR event is triggered, pmic_glink driver can invoke state_cb() for all of its clients if they've registered. With the following sequence happening occasionally, it can end up accessing an invalid bcdev pointer use-after-free. - Battery charger driver calling pmic_glink_register_client() - PDR event - pmic_glink driver invoking state_cb() for all clients - Battery charger driver schedules subsys_work - Battery charger driver calling pmic_glink_unregister_client() because battery_chg_register_panel_notifier() returns -EPROBE_DEFER - Battery charger driver's subsys work runs with an invalid bcdev pointer Fix this by moving battery_chg_register_panel_notifier() before calling pmic_glink_register_client(). With this, battery charger driver won't register with pmic_glink driver if it can't get an active panel from using of_drm_find_panel() on the boards where "qcom,display-panels" is specified. Also, update bcdev->state to PMIC_GLINK_STATE_UP only after pmic_glink_register_client() succeeds so that battery_chg_write() won't go through. In the driver probe error return path, cancel subsys_up_work to ensure that it's not invoked after pmic_glink_register_client() succeeds but an error path follows that. CRs-Fixed: 3274512 Fixes: 94f2a1746953 ("power: supply: qti_battery_charger: Disable notifications while in sleep") Change-Id: I12b9f9137602b760a751ad0119fce1cd10ba8fbd Signed-off-by: Subbaraman Narayanamurthy --- drivers/power/supply/qti_battery_charger.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/power/supply/qti_battery_charger.c b/drivers/power/supply/qti_battery_charger.c index 8ffc010fa0da..cbade3b2baf8 100644 --- a/drivers/power/supply/qti_battery_charger.c +++ b/drivers/power/supply/qti_battery_charger.c @@ -2190,9 +2190,12 @@ static int battery_chg_probe(struct platform_device *pdev) INIT_WORK(&bcdev->subsys_up_work, battery_chg_subsys_up_work); INIT_WORK(&bcdev->usb_type_work, battery_chg_update_usb_type_work); INIT_WORK(&bcdev->battery_check_work, battery_chg_check_status_work); - atomic_set(&bcdev->state, PMIC_GLINK_STATE_UP); bcdev->dev = dev; + rc = battery_chg_register_panel_notifier(bcdev); + if (rc < 0) + return rc; + client_data.id = MSG_OWNER_BC; client_data.name = "battery_charger"; client_data.msg_cb = battery_chg_callback; @@ -2208,6 +2211,7 @@ static int battery_chg_probe(struct platform_device *pdev) return rc; } + atomic_set(&bcdev->state, PMIC_GLINK_STATE_UP); bcdev->initialized = true; bcdev->reboot_notifier.notifier_call = battery_chg_ship_mode; bcdev->reboot_notifier.priority = 255; @@ -2219,10 +2223,6 @@ static int battery_chg_probe(struct platform_device *pdev) goto error; } - rc = battery_chg_register_panel_notifier(bcdev); - if (rc < 0) - goto error; - bcdev->restrict_fcc_ua = DEFAULT_RESTRICT_FCC_UA; platform_set_drvdata(pdev, bcdev); bcdev->fake_soc = -EINVAL; @@ -2247,6 +2247,7 @@ static int battery_chg_probe(struct platform_device *pdev) return 0; error: + cancel_work_sync(&bcdev->subsys_up_work); bcdev->initialized = false; complete(&bcdev->ack); pmic_glink_unregister_client(bcdev->client); @@ -2264,6 +2265,7 @@ static int battery_chg_remove(struct platform_device *pdev) device_init_wakeup(bcdev->dev, false); debugfs_remove_recursive(bcdev->debugfs_dir); + cancel_work_sync(&bcdev->subsys_up_work); class_unregister(&bcdev->battery_class); unregister_reboot_notifier(&bcdev->reboot_notifier); rc = pmic_glink_unregister_client(bcdev->client); From d0afae2596a6c0522e984bbca9aa1cc59d2e88c5 Mon Sep 17 00:00:00 2001 From: Subbaraman Narayanamurthy Date: Thu, 25 Aug 2022 10:31:08 -0700 Subject: [PATCH 2/2] power: supply: qti_battery_charger: Fix NULL pointer dereference With psy initialized in battery_chg_init_psy(), read_property_id() when called from battery_chg_parse_dt() can try accessing psy->desc->name when dynamic debugging is enabled. Fix it. Change-Id: I88114ea9779d5113524b8b15659bf8bafe6364ed Signed-off-by: Subbaraman Narayanamurthy --- drivers/power/supply/qti_battery_charger.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/power/supply/qti_battery_charger.c b/drivers/power/supply/qti_battery_charger.c index cbade3b2baf8..47bbdfc896da 100644 --- a/drivers/power/supply/qti_battery_charger.c +++ b/drivers/power/supply/qti_battery_charger.c @@ -411,8 +411,9 @@ static int write_property_id(struct battery_chg_dev *bcdev, req_msg.hdr.type = MSG_TYPE_REQ_RESP; req_msg.hdr.opcode = pst->opcode_set; - pr_debug("psy: %s prop_id: %u val: %u\n", pst->psy->desc->name, - req_msg.property_id, val); + if (pst->psy) + pr_debug("psy: %s prop_id: %u val: %u\n", pst->psy->desc->name, + req_msg.property_id, val); return battery_chg_write(bcdev, &req_msg, sizeof(req_msg)); } @@ -429,8 +430,9 @@ static int read_property_id(struct battery_chg_dev *bcdev, req_msg.hdr.type = MSG_TYPE_REQ_RESP; req_msg.hdr.opcode = pst->opcode_get; - pr_debug("psy: %s prop_id: %u\n", pst->psy->desc->name, - req_msg.property_id); + if (pst->psy) + pr_debug("psy: %s prop_id: %u\n", pst->psy->desc->name, + req_msg.property_id); return battery_chg_write(bcdev, &req_msg, sizeof(req_msg)); } @@ -444,8 +446,9 @@ static int get_property_id(struct psy_state *pst, if (pst->map[i] == prop) return i; - pr_err("No property id for property %d in psy %s\n", prop, - pst->psy->desc->name); + if (pst->psy) + pr_err("No property id for property %d in psy %s\n", prop, + pst->psy->desc->name); return -ENOENT; }