Re: [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published
From: Mukesh Ojha
Date: Tue Jun 30 2026 - 06:22:03 EST
On Tue, Jun 30, 2026 at 11:50:30AM +0200, Konrad Dybcio wrote:
> On 6/29/26 4:17 PM, Mukesh Ojha wrote:
> > In qcom_scm_probe(), devm_request_threaded_irq() is called before
> > smp_store_release(&__scm, scm). Two paths can dereference __scm before
> > it is published, both causing a NULL pointer dereference.
> >
> > The IRQ handler receives scm via its data argument but passes only wq_ctx
> > to qcom_scm_waitq_wakeup() and qcom_scm_get_completion(), which then
> > dereference __scm directly. Thread scm through both functions so the IRQ
> > handler path never touches __scm.
> >
> > Non-atomic SMC calls made during probe (e.g. from qcom_tzmem_init via
> > qcom_scm_shm_bridge_enable) can return WAITQ_SLEEP, causing
> > qcom_scm_wait_for_wq_completion() to run before __scm is published and
> > dereference it. Add platform_set_drvdata(pdev, scm) early in probe and
> > change qcom_scm_wait_for_wq_completion() to take the device pointer and
> > use dev_get_drvdata() to reach scm, removing any dependency on __scm.
> >
> > Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic")
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>
> > ---
>
> [...]
>
> > -int qcom_scm_wait_for_wq_completion(u32 wq_ctx)
> > +int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx)
>
> I think it logically makes more sense to pass a struct qcom_scm* here
> and get the data from the caller, but potayto/potato
To maintain the abstraction of struct qcom_scm* to
qcom_scm-smc.c, had to use a different way.
--
-Mukesh Ojha