| Message ID | 20240820-pd-mapper-refcount-fix-v1-1-03ea65c0309b@quicinc.com |
|---|---|
| State | Accepted |
| Commit | c158ceb826068a8bbe3c9e78df420f47ba53c8a8 |
| Headers | show |
| Series | soc: qcom: pd-mapper: Fix singleton refcount | expand |
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>
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,
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);
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(+) --- base-commit: 47ac09b91befbb6a235ab620c32af719f8208399 change-id: 20240820-pd-mapper-refcount-fix-404d290d6868 Best regards,