From: Bjorn Andersson <andersson@kernel.org>
To: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
Cc: Srinivas Kandagatla <srini@kernel.org>,
Amol Maheshwari <amahesh@qti.qualcomm.com>,
Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Thierry Escande <thierry.escande@linaro.org>,
linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH v2 1/2] misc: fastrpc: Fix NULL pointer dereference in rpmsg callback
Date: Mon, 4 May 2026 13:53:37 -0500 [thread overview]
Message-ID: <afjprOhBhP15-2lU@baldur> (raw)
In-Reply-To: <20260504171701.18164-1-mukesh.ojha@oss.qualcomm.com>
On Mon, May 04, 2026 at 10:47:00PM +0530, Mukesh Ojha wrote:
> A NULL pointer dereference was observed on Hawi at boot when the DSP
> sends a glink message before fastrpc_rpmsg_probe() has completed
> initialization:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000178
> pc : _raw_spin_lock_irqsave+0x34/0x8c
> lr : fastrpc_rpmsg_callback+0x3c/0xcc [fastrpc]
> ...
> Call trace:
> _raw_spin_lock_irqsave+0x34/0x8c (P)
> fastrpc_rpmsg_callback+0x3c/0xcc [fastrpc]
> qcom_glink_native_rx+0x538/0x6a4
> qcom_glink_smem_intr+0x14/0x24 [qcom_glink_smem]
>
> The faulting address 0x178 corresponds to the lock variable inside
> struct fastrpc_channel_ctx, confirming that cctx is NULL when
> fastrpc_rpmsg_callback() attempts to take the spinlock.
>
> There are two issues here. First, dev_set_drvdata() is called before
> spin_lock_init() and idr_init(), leaving a window where the callback
> can retrieve a valid cctx pointer but operate on an uninitialized
> spinlock. Second, the rpmsg channel becomes live as soon as the driver
> is bound, so fastrpc_rpmsg_callback() can fire before dev_set_drvdata()
> is called at all, resulting in dev_get_drvdata() returning NULL.
>
> Fix both issues by moving all cctx initialization ahead of
> dev_set_drvdata() so the structure is fully initialized before it
> becomes visible to the callback, and add a NULL check in
> fastrpc_rpmsg_callback() as a guard against any remaining window.
>
> Fixes: f6f9279f2bf0 ("misc: fastrpc: Add Qualcomm fastrpc basic driver model")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
The fix looks good to me.
Reviewed-by: Bjorn Andersson <andersson@kernel.org>
But I can't help wonder, what's in that message? Should we make sure to
handle it, longer term?
Regards,
Bjorn
> ---
> Changes in v2: https://lore.kernel.org/lkml/20260417200146.184425-1-mukesh.ojha@oss.qualcomm.com/
> - Added stable mailing list and fixes tag.
>
> drivers/misc/fastrpc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 1080f9acf70a..a1a54453bb7e 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2431,7 +2431,6 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>
> kref_init(&data->refcount);
>
> - dev_set_drvdata(&rpdev->dev, data);
> rdev->dma_mask = &data->dma_mask;
> dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
> INIT_LIST_HEAD(&data->users);
> @@ -2440,6 +2439,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> idr_init(&data->ctx_idr);
> data->domain_id = domain_id;
> data->rpdev = rpdev;
> + dev_set_drvdata(&rpdev->dev, data);
>
> err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
> if (err)
> @@ -2513,6 +2513,9 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
> if (len < sizeof(*rsp))
> return -EINVAL;
>
> + if (!cctx)
> + return -ENODEV;
> +
> ctxid = ((rsp->ctx & FASTRPC_CTXID_MASK) >> 4);
>
> spin_lock_irqsave(&cctx->lock, flags);
> --
> 2.53.0
>
>
prev parent reply other threads:[~2026-05-04 18:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-04 17:17 [PATCH v2 1/2] misc: fastrpc: Fix NULL pointer dereference in rpmsg callback Mukesh Ojha
2026-05-04 17:17 ` [PATCH v2 2/2] misc: fastrpc: Move prints outside spinlock in fastrpc_cb_probe Mukesh Ojha
2026-05-04 18:54 ` Bjorn Andersson
2026-05-04 18:53 ` Bjorn Andersson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afjprOhBhP15-2lU@baldur \
--to=andersson@kernel.org \
--cc=amahesh@qti.qualcomm.com \
--cc=arnd@arndb.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mukesh.ojha@oss.qualcomm.com \
--cc=srini@kernel.org \
--cc=stable@vger.kernel.org \
--cc=thierry.escande@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox