1efdbf53 | 20-Aug-2024 |
Bjorn Andersson <quic_bjorande@quicinc.com> |
soc: qcom: pmic_glink: Fix race during initialization
commit 3568affcddd68743e25aa3ec1647d9b82797757b upstream.
As pointed out by Stephen Boyd it is possible that during initialization of the pmic_
soc: qcom: pmic_glink: Fix race during initialization
commit 3568affcddd68743e25aa3ec1647d9b82797757b upstream.
As pointed out by Stephen Boyd it is possible that during initialization of the pmic_glink child drivers, the protection-domain notifiers fires, and the associated work is scheduled, before the client registration returns and as a result the local "client" pointer has been initialized.
The outcome of this is a NULL pointer dereference as the "client" pointer is blindly dereferenced.
Timeline provided by Stephen: CPU0 CPU1 ---- ---- ucsi->client = NULL; devm_pmic_glink_register_client() client->pdr_notify(client->priv, pg->client_state) pmic_glink_ucsi_pdr_notify() schedule_work(&ucsi->register_work) <schedule away> pmic_glink_ucsi_register() ucsi_register() pmic_glink_ucsi_read_version() pmic_glink_ucsi_read() pmic_glink_ucsi_read() pmic_glink_send(ucsi->client) <client is NULL BAD> ucsi->client = client // Too late!
This code is identical across the altmode, battery manager and usci child drivers.
Resolve this by splitting the allocation of the "client" object and the registration thereof into two operations.
This only happens if the protection domain registry is populated at the time of registration, which by the introduction of commit '1ebcde047c54 ("soc: qcom: add pd-mapper implementation")' became much more likely.
Reported-by: Amit Pundir <amit.pundir@linaro.org> Closes: https://lore.kernel.org/all/CAMi1Hd2_a7TjA7J9ShrAbNOd_CoZ3D87twmO5t+nZxC9sX18tA@mail.gmail.com/ Reported-by: Johan Hovold <johan@kernel.org> Closes: https://lore.kernel.org/all/ZqiyLvP0gkBnuekL@hovoldconsulting.com/ Reported-by: Stephen Boyd <swboyd@chromium.org> Closes: https://lore.kernel.org/all/CAE-0n52JgfCBWiFQyQWPji8cq_rCsviBpW-m72YitgNfdaEhQg@mail.gmail.com/ Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK driver") Cc: stable@vger.kernel.org Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> Tested-by: Amit Pundir <amit.pundir@linaro.org> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com> Tested-by: Johan Hovold <johan+linaro@kernel.org> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> Link: https://lore.kernel.org/r/20240820-pmic-glink-v6-11-races-v3-1-eec53c750a04@quicinc.com Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
c704091b | 20-Aug-2024 |
Bjorn Andersson <quic_bjorande@quicinc.com> |
soc: qcom: pmic_glink: Actually communicate when remote goes down
commit ad51126037a43c05f5f4af5eb262734e3e88ca59 upstream.
When the pmic_glink state is UP and we either receive a protection- domai
soc: qcom: pmic_glink: Actually communicate when remote goes down
commit ad51126037a43c05f5f4af5eb262734e3e88ca59 upstream.
When the pmic_glink state is UP and we either receive a protection- domain (PD) notification indicating that the PD is going down, or that the whole remoteproc is going down, it's expected that the pmic_glink client instances are notified that their function has gone DOWN.
This is not what the code does, which results in the client state either not updating, or being wrong in many cases. So let's fix the conditions.
Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK driver") Cc: stable@vger.kernel.org Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Tested-by: Amit Pundir <amit.pundir@linaro.org> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Tested-by: Johan Hovold <johan+linaro@kernel.org> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> Link: https://lore.kernel.org/r/20240820-pmic-glink-v6-11-races-v3-3-eec53c750a04@quicinc.com Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
show more ...
|
d48f3bb4 | 21-Jun-2024 |
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> |
soc: qcom: pdr: fix parsing of domains lists
[ Upstream commit 57f20d51f35780f240ecf39d81cda23612800a92 ]
While parsing the domains list, start offsets from 0 rather than from domains_read. The dom
soc: qcom: pdr: fix parsing of domains lists
[ Upstream commit 57f20d51f35780f240ecf39d81cda23612800a92 ]
While parsing the domains list, start offsets from 0 rather than from domains_read. The domains_read is equal to the total count of the domains we have seen, while the domains list in the message starts from offset 0.
Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers") Tested-by: Steev Klimaszewski <steev@kali.org> Tested-by: Alexey Minnekhanov <alexeymin@postmarketos.org> Reviewed-by: Chris Lew <quic_clew@quicinc.com> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550-QRD Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Link: https://lore.kernel.org/r/20240622-qcom-pd-mapper-v9-2-a84ee3591c8e@linaro.org Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
3e815626 | 21-Jun-2024 |
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> |
soc: qcom: pdr: protect locator_addr with the main mutex
[ Upstream commit 107924c14e3ddd85119ca43c26a4ee1056fa9b84 ]
If the service locator server is restarted fast enough, the PDR can rewrite loc
soc: qcom: pdr: protect locator_addr with the main mutex
[ Upstream commit 107924c14e3ddd85119ca43c26a4ee1056fa9b84 ]
If the service locator server is restarted fast enough, the PDR can rewrite locator_addr fields concurrently. Protect them by placing modification of those fields under the main pdr->lock.
Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers") Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8550-QRD Tested-by: Steev Klimaszewski <steev@kali.org> Tested-by: Alexey Minnekhanov <alexeymin@postmarketos.org> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Link: https://lore.kernel.org/r/20240622-qcom-pd-mapper-v9-1-a84ee3591c8e@linaro.org Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
aad41f4c | 13-Jun-2024 |
Sibi Sankar <quic_sibis@quicinc.com> |
soc: qcom: icc-bwmon: Fix refcount imbalance seen during bwmon_remove
[ Upstream commit 24086640ab39396eb1a92d1cb1cd2f31b2677c52 ]
The following warning is seen during bwmon_remove due to refcount
soc: qcom: icc-bwmon: Fix refcount imbalance seen during bwmon_remove
[ Upstream commit 24086640ab39396eb1a92d1cb1cd2f31b2677c52 ]
The following warning is seen during bwmon_remove due to refcount imbalance, fix this by releasing the OPPs after use.
Logs: WARNING: at drivers/opp/core.c:1640 _opp_table_kref_release+0x150/0x158 Hardware name: Qualcomm Technologies, Inc. X1E80100 CRD (DT) ... Call trace: _opp_table_kref_release+0x150/0x158 dev_pm_opp_remove_table+0x100/0x1b4 devm_pm_opp_of_table_release+0x10/0x1c devm_action_release+0x14/0x20 devres_release_all+0xa4/0x104 device_unbind_cleanup+0x18/0x60 device_release_driver_internal+0x1ec/0x228 driver_detach+0x50/0x98 bus_remove_driver+0x6c/0xbc driver_unregister+0x30/0x60 platform_driver_unregister+0x14/0x20 bwmon_driver_exit+0x18/0x524 [icc_bwmon] __arm64_sys_delete_module+0x184/0x264 invoke_syscall+0x48/0x118 el0_svc_common.constprop.0+0xc8/0xe8 do_el0_svc+0x20/0x2c el0_svc+0x34/0xdc el0t_64_sync_handler+0x13c/0x158 el0t_64_sync+0x190/0x194 --[ end trace 0000000000000000 ]---
Fixes: 0276f69f13e2 ("soc: qcom: icc-bwmon: Set default thresholds dynamically") Fixes: b9c2ae6cac40 ("soc: qcom: icc-bwmon: Add bandwidth monitoring driver") Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Link: https://lore.kernel.org/r/20240613164506.982068-1-quic_sibis@quicinc.com Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
459f84f8 | 09-May-2024 |
Stephen Boyd <swboyd@chromium.org> |
soc: qcom: rpmh-rsc: Ensure irqs aren't disabled by rpmh_rsc_send_data() callers
[ Upstream commit e43111f52b9ec5c2d700f89a1d61c8d10dc2d9e9 ]
Dan pointed out that Smatch is concerned about this cod
soc: qcom: rpmh-rsc: Ensure irqs aren't disabled by rpmh_rsc_send_data() callers
[ Upstream commit e43111f52b9ec5c2d700f89a1d61c8d10dc2d9e9 ]
Dan pointed out that Smatch is concerned about this code because it uses spin_lock_irqsave() and then calls wait_event_lock_irq() which enables irqs before going to sleep. The comment above the function says it should be called with interrupts enabled, but we simply hope that's true without really confirming that. Let's add a might_sleep() here to confirm that interrupts and preemption aren't disabled. Once we do that, we can change the lock to be non-saving, spin_lock_irq(), to clarify that we don't expect irqs to be disabled. If irqs are disabled by callers they're going to be enabled anyway in the wait_event_lock_irq() call which would be bad.
This should make Smatch happier and find bad callers faster with the might_sleep(). We can drop the WARN_ON() in the caller because we have the might_sleep() now, simplifying the code.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/r/911181ed-c430-4592-ad26-4dc948834e08@moroto.mountain Fixes: 2bc20f3c8487 ("soc: qcom: rpmh-rsc: Sleep waiting for tcs slots to be free") Cc: Douglas Anderson <dianders@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20240509184129.3924422-1-swboyd@chromium.org Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
fbadcde1 | 30-Apr-2024 |
Bjorn Andersson <quic_bjorande@quicinc.com> |
soc: qcom: pmic_glink: Make client-lock non-sleeping
[ Upstream commit 9329933699b32d467a99befa20415c4b2172389a ]
The recently introduced commit '635ce0db8956 ("soc: qcom: pmic_glink: don't travers
soc: qcom: pmic_glink: Make client-lock non-sleeping
[ Upstream commit 9329933699b32d467a99befa20415c4b2172389a ]
The recently introduced commit '635ce0db8956 ("soc: qcom: pmic_glink: don't traverse clients list without a lock")' ensured that the clients list is not modified while traversed.
But the callback is made from the GLINK IRQ handler and as such this mutual exclusion can not be provided by a (sleepable) mutex.
Replace the mutex with a spinlock.
Fixes: 635ce0db8956 ("soc: qcom: pmic_glink: don't traverse clients list without a lock") Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Link: https://lore.kernel.org/r/20240430-pmic-glink-sleep-while-atomic-v1-1-88fb493e8545@quicinc.com Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
8fc79346 | 02-Apr-2024 |
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> |
soc: qcom: pmic_glink: notify clients about the current state
[ Upstream commit d6cbce2cd354c9a37a558f290a8f1dfd20584f99 ]
In case the client is registered after the pmic-glink recived a response f
soc: qcom: pmic_glink: notify clients about the current state
[ Upstream commit d6cbce2cd354c9a37a558f290a8f1dfd20584f99 ]
In case the client is registered after the pmic-glink recived a response from the Protection Domain mapper, it is going to miss the notification about the state. Notify clients about the current state upon registration.
Fixes: 58ef4ece1e41 ("soc: qcom: pmic_glink: Introduce base PMIC GLINK driver") Reviewed-by: Andrew Halaney <ahalaney@redhat.com> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com> Tested-by: Xilin Wu <wuxilin123@gmail.com> # on QCS8550 AYN Odin 2 Link: https://lore.kernel.org/r/20240403-pmic-glink-fix-clients-v2-2-aed4e02baacc@linaro.org Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|
004e05c2 | 12-Oct-2023 |
Abel Vesa <abel.vesa@linaro.org> |
soc: qcom: llcc: Fix LLCC_TRP_ATTR2_CFGn offset
[ Upstream commit 110cb8d861cc1a040cdab495b22ac436c49d1454 ]
According to documentation, it has increments of 4, not 8.
Fixes: c72ca343f911 ("soc: q
soc: qcom: llcc: Fix LLCC_TRP_ATTR2_CFGn offset
[ Upstream commit 110cb8d861cc1a040cdab495b22ac436c49d1454 ]
According to documentation, it has increments of 4, not 8.
Fixes: c72ca343f911 ("soc: qcom: llcc: Add v4.1 HW version support") Reported-by: Unnathi Chalicheemala <quic_uchalich@quicinc.com> Reviewed-by: Satya Durga Srinivasu Prabhala <quic_satyap@quicinc.com> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> Link: https://lore.kernel.org/r/20231012160509.184891-1-abel.vesa@linaro.org Signed-off-by: Bjorn Andersson <andersson@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
show more ...
|