[PATCH] soc: qcom: pd-mapper: Fix singleton refcount

[PATCH] soc: qcom: pd-mapper: Fix singleton refcount

[Bjorn Andersson]

The Qualcomm pd-mapper is a refcounted singleton, but the refcount is
never incremented, which means the as soon as any remoteproc instance
stops the count will hit 0.

At this point the pd-mapper QMI service is stopped, leaving firmware
without access to the PD information. Stopping any other remoteproc
instances will result in a use-after-free, which best case manifest
itself as a refcount underflow:

  refcount_t: underflow; use-after-free.
  WARNING: CPU: 1 PID: 354 at lib/refcount.c:87 refcount_dec_and_mutex_lock+0xc4/0x148
  ...
  Call trace:
   refcount_dec_and_mutex_lock+0xc4/0x148
   qcom_pdm_remove+0x40/0x118 [qcom_pd_mapper]
   ...

Fix this by incrementing the refcount, so that the pd-mapper is only
torn down when the last remoteproc stops, as intended.

Fixes: 1ebcde047c54 ("soc: qcom: add pd-mapper implementation")
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/soc/qcom/qcom_pd_mapper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/soc/qcom/qcom_pd_mapper.c b/drivers/soc/qcom/qcom_pd_mapper.c
index a4c007080665..d6ddf71e83bb 100644
--- a/drivers/soc/qcom/qcom_pd_mapper.c
+++ b/drivers/soc/qcom/qcom_pd_mapper.c
@@ -635,6 +635,8 @@ static int qcom_pdm_probe(struct auxiliary_device *auxdev,
 			ret = PTR_ERR(data);
 		else
 			__qcom_pdm_data = data;
+	} else {
+		refcount_inc(&__qcom_pdm_data->refcnt);
 	}
 
 	auxiliary_set_drvdata(auxdev, __qcom_pdm_data);

---
base-commit: 47ac09b91befbb6a235ab620c32af719f8208399
change-id: 20240820-pd-mapper-refcount-fix-404d290d6868

Best regards,
-- 
Bjorn Andersson <quic_bjorande@quicinc.com>

Re: [PATCH] soc: qcom: pd-mapper: Fix singleton refcount

[Dmitry Baryshkov]

On August 21, 2024 5:59:34 AM GMT+07:00, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>The Qualcomm pd-mapper is a refcounted singleton, but the refcount is
>never incremented, which means the as soon as any remoteproc instance
>stops the count will hit 0.
>
>At this point the pd-mapper QMI service is stopped, leaving firmware
>without access to the PD information. Stopping any other remoteproc
>instances will result in a use-after-free, which best case manifest
>itself as a refcount underflow:
>
>  refcount_t: underflow; use-after-free.
>  WARNING: CPU: 1 PID: 354 at lib/refcount.c:87 refcount_dec_and_mutex_lock+0xc4/0x148
>  ...
>  Call trace:
>   refcount_dec_and_mutex_lock+0xc4/0x148
>   qcom_pdm_remove+0x40/0x118 [qcom_pd_mapper]
>   ...
>
>Fix this by incrementing the refcount, so that the pd-mapper is only
>torn down when the last remoteproc stops, as intended.
>
>Fixes: 1ebcde047c54 ("soc: qcom: add pd-mapper implementation")
>Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>---
> drivers/soc/qcom/qcom_pd_mapper.c | 2 ++
> 1 file changed, 2 insertions(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>




-- 
With best wishes
Dmitry

Re: [PATCH] soc: qcom: pd-mapper: Fix singleton refcount

[Bjorn Andersson]

On Tue, 20 Aug 2024 15:59:34 -0700, Bjorn Andersson wrote:
> The Qualcomm pd-mapper is a refcounted singleton, but the refcount is
> never incremented, which means the as soon as any remoteproc instance
> stops the count will hit 0.
> 
> At this point the pd-mapper QMI service is stopped, leaving firmware
> without access to the PD information. Stopping any other remoteproc
> instances will result in a use-after-free, which best case manifest
> itself as a refcount underflow:
> 
> [...]

Applied, thanks!

[1/1] soc: qcom: pd-mapper: Fix singleton refcount
      commit: c158ceb826068a8bbe3c9e78df420f47ba53c8a8

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>