linux-kernel - Re: [PATCH] Revert "power: supply: qcom_battmgr: Register the power supplies after PDR is up"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e7457703-fcd1-448a-bcb1-d5aa31bbb1fc@linaro.org>
Date: Thu, 25 Jan 2024 00:35:20 +0100
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Johan Hovold <johan+linaro@...nel.org>, Sebastian Reichel <sre@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
 Marijn Suijten <marijn.suijten@...ainline.org>,
 Neil Armstrong <neil.armstrong@...aro.org>, Xilin Wu <wuxilin123@...il.com>,
 linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Revert "power: supply: qcom_battmgr: Register the power
 supplies after PDR is up"



On 1/23/24 17:00, Johan Hovold wrote:
> This reverts commit b43f7ddc2b7a5a90447d96cb4d3c6d142dd4a810.
> 
> The offending commit deferred power-supply class device registration
> until the service-started notification is received.
> 
> This triggers a NULL pointer dereference during boot of the Lenovo
> ThinkPad X13s and SC8280XP CRD as battery status notifications can be
> received before the service-start notification:
> 
> 	Unable to handle kernel NULL pointer dereference at virtual address 00000000000005c0
> 	...
> 	Call trace:
> 	 _acquire+0x338/0x2064
> 	 acquire+0x1e8/0x318
> 	 spin_lock_irqsave+0x60/0x88
> 	 _supply_changed+0x2c/0xa4
> 	 battmgr_callback+0x1d4/0x60c [qcom_battmgr]
> 	 pmic_glink_rpmsg_callback+0x5c/0xa4 [pmic_glink]
> 	 qcom_glink_native_rx+0x58c/0x7e8
> 	 qcom_glink_smem_intr+0x14/0x24 [qcom_glink_smem]
> 	 __handle_irq_event_percpu+0xb0/0x2d4
> 	 handle_irq_event+0x4c/0xb8
> 
> As trying to serialise this is non-trivial and risks missing
> notifications, let's revert to registration during probe so that the
> driver data is all set up once the service goes live.
> 
> The warning message during resume in case the aDSP firmware is not
> running that motivated the change can be considered a feature and should
> not be suppressed.
> 
> Fixes: b43f7ddc2b7a ("power: supply: qcom_battmgr: Register the power supplies after PDR is up")
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
> ---

Johan and I talked off-list and decided reverting this is the best
way to go forward for now, the problem that my original commit
solved (or well, tried to, anyway) is only an issue with a
configuration that's not quite supported (i.e. missing all
pieces of the puzzle for a functional battmgr) and more work would
need to be put into supporting that. I *may* do it in the future,
but for now both of us need to work on different things.

Reviewed-by: Konrad Dybcio <konrad.dybcio@...aro.org>

Konrad