mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] fastrpc: fix probe-time races between rpmsg and userspace
@ 2026-10-01 11:01 Vinayak Katoch
  2026-10-01 11:01 ` [PATCH 1/2] misc: fastrpc: fix NULL rpdev dereference when session alloc fails Vinayak Katoch
  2026-10-01 11:01 ` [PATCH 2/2] misc: fastrpc: fix init ordering race in rpmsg probe Vinayak Katoch
  0 siblings, 2 replies; 5+ messages in thread
From: Vinayak Katoch @ 2026-10-01 11:01 UTC (permalink / raw)
  To: Srinivas Kandagatla, Ekansh Gupta, Arnd Bergmann,
	Greg Kroah-Hartman, Thierry Escande
  Cc: Bharath Kumar, Chenna Kesava Raju, linux-arm-msm, dri-devel,
	linux-kernel, Vinayak Katoch

This series fixes two races in fastrpc_rpmsg_probe() that can be
triggered when userspace opens a fastrpc device node concurrently with
the rpmsg probe sequence.

fastrpc_device_register() calls misc_register() before the channel
context is fully initialised. A concurrent open() in that window hits
kref_get() on a zero refcount, and because fastrpc_cb_devices_create()
has not yet run sesscount is 0, the error path calls kref_put() on the
saturated refcount, triggering kfree() of the channel context. A
subsequent IRQ-path fastrpc_rpmsg_callback() then dereferences the
freed spinlock, causing a kernel panic. A NULL dereference in the
fastrpc_device_open() error path when cctx->rpdev is concurrently
cleared by fastrpc_rpmsg_remove() is also addressed.

Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
Vinayak Katoch (2):
      misc: fastrpc: fix NULL rpdev dereference when session alloc fails
      misc: fastrpc: fix init ordering race in rpmsg probe

 drivers/misc/fastrpc.c | 55 ++++++++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 26 deletions(-)
---
base-commit: 5c4d4169604b335c38bbc79bc1fc03042981fc6f
change-id: 20260923-fastrpc-probe-fixes-30c1980b50e6
prerequisite-change-id: 20260609-dup-sessions-ea2acaac1994:v5
prerequisite-patch-id: 425dc9414848bbc3f2a053d067195d849898e2ae
prerequisite-patch-id: 4fea14b47d11a5a3e18065d8cf3a461841ee2b86
prerequisite-patch-id: da0a3d55397a522ea4b77769e387df9faea8e024

Best regards,
-- 
Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] misc: fastrpc: fix NULL rpdev dereference when session alloc fails
  2026-10-01 11:01 [PATCH 0/2] fastrpc: fix probe-time races between rpmsg and userspace Vinayak Katoch
@ 2026-10-01 11:01 ` Vinayak Katoch
  2026-10-14  4:02   ` Ekansh Gupta
  2026-10-01 11:01 ` [PATCH 2/2] misc: fastrpc: fix init ordering race in rpmsg probe Vinayak Katoch
  1 sibling, 1 reply; 5+ messages in thread
From: Vinayak Katoch @ 2026-10-01 11:01 UTC (permalink / raw)
  To: Srinivas Kandagatla, Ekansh Gupta, Arnd Bergmann,
	Greg Kroah-Hartman, Thierry Escande
  Cc: Bharath Kumar, Chenna Kesava Raju, linux-arm-msm, dri-devel,
	linux-kernel, Vinayak Katoch

When a fastrpc client process dies abnormally without closing its file
descriptor, fastrpc_device_release() is never called and the compute-cb
session slot is never freed. The abnormal client death on the DSP side
triggers a glink channel teardown which propagates up through the rpmsg
bus and calls fastrpc_rpmsg_remove().

fastrpc_rpmsg_remove() nulls cctx->rpdev before calling
misc_deregister(). If a concurrent open() enters fastrpc_device_open()
in this window and fastrpc_session_alloc() returns NULL (sessions
exhausted by the leaked slot), the error path dereferences
cctx->rpdev->dev causing a NULL pointer dereference:

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
  pc : fastrpc_device_open+0x1b8/0x258 [fastrpc]
  Call trace:
   fastrpc_device_open+0x1b8/0x258 [fastrpc] (P)
   misc_open+0xd8/0x1a0

Fix by moving misc_deregister() before rpdev = NULL. misc_open() and
misc_deregister() both take misc_mtx, so misc_deregister() will either
block until fastrpc_device_open() completes or prevent new opens from
entering it entirely.

Fixes: 7c11df42d0c7 ("misc: fastrpc: Fix device_open when no session is available")
Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index b29c1fd00de2..7ab1cbbf19bb 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -2706,6 +2706,12 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 	struct fastrpc_user *user;
 	unsigned long flags;
 
+	if (cctx->fdevice)
+		misc_deregister(&cctx->fdevice->miscdev);
+
+	if (cctx->secure_fdevice)
+		misc_deregister(&cctx->secure_fdevice->miscdev);
+
 	/* No invocations past this point */
 	spin_lock_irqsave(&cctx->lock, flags);
 	cctx->rpdev = NULL;
@@ -2713,12 +2719,6 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 		fastrpc_notify_users(user);
 	spin_unlock_irqrestore(&cctx->lock, flags);
 
-	if (cctx->fdevice)
-		misc_deregister(&cctx->fdevice->miscdev);
-
-	if (cctx->secure_fdevice)
-		misc_deregister(&cctx->secure_fdevice->miscdev);
-
 	list_for_each_entry_safe(buf, b, &cctx->invoke_interrupted_mmaps, node)
 		list_del(&buf->node);
 

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] misc: fastrpc: fix init ordering race in rpmsg probe
  2026-10-01 11:01 [PATCH 0/2] fastrpc: fix probe-time races between rpmsg and userspace Vinayak Katoch
  2026-10-01 11:01 ` [PATCH 1/2] misc: fastrpc: fix NULL rpdev dereference when session alloc fails Vinayak Katoch
@ 2026-10-01 11:01 ` Vinayak Katoch
  2026-10-14  4:10   ` Ekansh Gupta
  1 sibling, 1 reply; 5+ messages in thread
From: Vinayak Katoch @ 2026-10-01 11:01 UTC (permalink / raw)
  To: Srinivas Kandagatla, Ekansh Gupta, Arnd Bergmann,
	Greg Kroah-Hartman, Thierry Escande
  Cc: Bharath Kumar, Chenna Kesava Raju, linux-arm-msm, dri-devel,
	linux-kernel, Vinayak Katoch

fastrpc_device_register() calls misc_register() which makes the device
node visible to userspace. However, kref_init(), spin_lock_init(),
INIT_LIST_HEAD(), dev_set_drvdata() and fastrpc_cb_devices_create()
were all called after the switch/misc_register block, leaving a window
where userspace can open the device and call fastrpc_device_open() on
a partially initialised channel context.

On a concurrent open(), fastrpc_channel_ctx_get() calls kref_get() on
a kzalloc-zeroed, never kref_init()'d refcount of 0, triggering:

  refcount_t: addition on 0; use-after-free.
  Call trace:
   refcount_warn_saturate+0x120/0x148 (P)
   fastrpc_device_open+0x204/0x258 [fastrpc]
   misc_open+0xd8/0x1a0

Additionally, fastrpc_cb_devices_create() not having run means
sesscount is still 0, so fastrpc_session_alloc() returns NULL and
fastrpc_device_open() hits the error path which calls
fastrpc_channel_ctx_put() on the already-bogus refcount. This
triggers the free callback, kfree()-ing the channel context. A
subsequent IRQ-path fastrpc_rpmsg_callback() then dereferences the
freed spinlock:

  Unable to handle kernel paging request at virtual address ffffcd5841510ca0
  Internal error: Oops: 0000000096000047 [#1] SMP
  Call trace:
   queued_spin_lock_slowpath+0x414/0x5e0 (P)
   _raw_spin_lock_irqsave+0x74/0x90
   fastrpc_rpmsg_callback+0x4c/0xf0 [fastrpc]
   qcom_glink_native_rx+0x6c4/0xf10
   qcom_glink_smem_intr+0x1c/0x38 [qcom_glink_smem]
  Kernel panic - not syncing: Oops: Fatal exception in interrupt

Fix by moving all channel context initialisation (kref_init,
spin_lock_init, INIT_LIST_HEAD, idr_init, dev_set_drvdata) and
fastrpc_cb_devices_create() before fastrpc_device_register(), ensuring
the struct is fully ready before the device node is visible to
userspace.

Add err_depopulate label in the error path since
fastrpc_cb_devices_create() now runs before misc_register(); a
fastrpc_device_register() failure must call fastrpc_cb_devices_destroy()
to clean up the compute-cb children before freeing data.

Fixes: 3abe3ab3cdab ("misc: fastrpc: add secure domain support")
Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 7ab1cbbf19bb..93d07d28a0f1 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -2614,6 +2614,23 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 	data->poll_mode_supported = soc_data->poll_mode_supported ||
 		of_machine_get_match(fastrpc_poll_supported_machines);
 
+	kref_init(&data->refcount);
+	atomic_set(&data->ctx_seq, 0);
+
+	rdev->dma_mask = &data->dma_mask;
+	dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
+	INIT_LIST_HEAD(&data->users);
+	INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
+	spin_lock_init(&data->lock);
+	idr_init(&data->ctx_idr);
+	data->domain_id = domain_id;
+	data->rpdev = rpdev;
+	dev_set_drvdata(&rpdev->dev, data);
+
+	err = fastrpc_cb_devices_create(rpdev);
+	if (err)
+		goto err_free_data;
+
 	switch (domain_id) {
 	case ADSP_DOMAIN_ID:
 	case MDSP_DOMAIN_ID:
@@ -2622,7 +2639,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 		data->unsigned_support = false;
 		err = fastrpc_device_register(rdev, data, secure_dsp, domain);
 		if (err)
-			goto err_free_data;
+			goto err_depopulate;
 		break;
 	case CDSP_DOMAIN_ID:
 	case GDSP_DOMAIN_ID:
@@ -2630,7 +2647,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 		/* Create both device nodes so that we can allow both Signed and Unsigned PD */
 		err = fastrpc_device_register(rdev, data, true, domain);
 		if (err)
-			goto err_free_data;
+			goto err_depopulate;
 
 		err = fastrpc_device_register(rdev, data, false, domain);
 		if (err)
@@ -2638,26 +2655,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 		break;
 	default:
 		err = -EINVAL;
-		goto err_free_data;
+		goto err_depopulate;
 	}
 
-	kref_init(&data->refcount);
-	atomic_set(&data->ctx_seq, 0);
-
-	rdev->dma_mask = &data->dma_mask;
-	dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
-	INIT_LIST_HEAD(&data->users);
-	INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
-	spin_lock_init(&data->lock);
-	idr_init(&data->ctx_idr);
-	data->domain_id = domain_id;
-	data->rpdev = rpdev;
-	dev_set_drvdata(&rpdev->dev, data);
-
-	err = fastrpc_cb_devices_create(rpdev);
-	if (err)
-		goto err_deregister_fdev;
-
 	if (data->domain_id == ADSP_DOMAIN_ID && data->sesscount > 0) {
 		struct fastrpc_session_ctx *last_sess;
 		struct fastrpc_session_ctx *dup_sess;
@@ -2682,6 +2682,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 	if (data->secure_fdevice)
 		misc_deregister(&data->secure_fdevice->miscdev);
 
+err_depopulate:
+	fastrpc_cb_devices_destroy(rpdev);
+
 err_free_data:
 	kfree(data);
 	return err;

-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] misc: fastrpc: fix NULL rpdev dereference when session alloc fails
  2026-10-01 11:01 ` [PATCH 1/2] misc: fastrpc: fix NULL rpdev dereference when session alloc fails Vinayak Katoch
@ 2026-10-14  4:02   ` Ekansh Gupta
  0 siblings, 0 replies; 5+ messages in thread
From: Ekansh Gupta @ 2026-10-14  4:02 UTC (permalink / raw)
  To: Vinayak Katoch, Srinivas Kandagatla, Arnd Bergmann,
	Greg Kroah-Hartman, Thierry Escande
  Cc: Bharath Kumar, Chenna Kesava Raju, linux-arm-msm, dri-devel,
	linux-kernel

On 01-10-2026 16:31, Vinayak Katoch wrote:
> When a fastrpc client process dies abnormally without closing its file
> descriptor, fastrpc_device_release() is never called and the compute-cb
> session slot is never freed. The abnormal client death on the DSP side
> triggers a glink channel teardown which propagates up through the rpmsg
> bus and calls fastrpc_rpmsg_remove().
> 
> fastrpc_rpmsg_remove() nulls cctx->rpdev before calling
> misc_deregister(). If a concurrent open() enters fastrpc_device_open()
> in this window and fastrpc_session_alloc() returns NULL (sessions
> exhausted by the leaked slot), the error path dereferences
> cctx->rpdev->dev causing a NULL pointer dereference:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
>   pc : fastrpc_device_open+0x1b8/0x258 [fastrpc]
>   Call trace:
>    fastrpc_device_open+0x1b8/0x258 [fastrpc] (P)
>    misc_open+0xd8/0x1a0
> 
> Fix by moving misc_deregister() before rpdev = NULL. misc_open() and
> misc_deregister() both take misc_mtx, so misc_deregister() will either
> block until fastrpc_device_open() completes or prevent new opens from
> entering it entirely.
> 
> Fixes: 7c11df42d0c7 ("misc: fastrpc: Fix device_open when no session is available")
> Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>

Reviewed-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] misc: fastrpc: fix init ordering race in rpmsg probe
  2026-10-01 11:01 ` [PATCH 2/2] misc: fastrpc: fix init ordering race in rpmsg probe Vinayak Katoch
@ 2026-10-14  4:10   ` Ekansh Gupta
  0 siblings, 0 replies; 5+ messages in thread
From: Ekansh Gupta @ 2026-10-14  4:10 UTC (permalink / raw)
  To: Vinayak Katoch, Srinivas Kandagatla, Arnd Bergmann,
	Greg Kroah-Hartman, Thierry Escande
  Cc: Bharath Kumar, Chenna Kesava Raju, linux-arm-msm, dri-devel,
	linux-kernel

On 01-10-2026 16:31, Vinayak Katoch wrote:
> fastrpc_device_register() calls misc_register() which makes the device
> node visible to userspace. However, kref_init(), spin_lock_init(),
> INIT_LIST_HEAD(), dev_set_drvdata() and fastrpc_cb_devices_create()
> were all called after the switch/misc_register block, leaving a window
> where userspace can open the device and call fastrpc_device_open() on
> a partially initialised channel context.
> 
> On a concurrent open(), fastrpc_channel_ctx_get() calls kref_get() on
> a kzalloc-zeroed, never kref_init()'d refcount of 0, triggering:
> 
>   refcount_t: addition on 0; use-after-free.
>   Call trace:
>    refcount_warn_saturate+0x120/0x148 (P)
>    fastrpc_device_open+0x204/0x258 [fastrpc]
>    misc_open+0xd8/0x1a0
> 
> Additionally, fastrpc_cb_devices_create() not having run means
> sesscount is still 0, so fastrpc_session_alloc() returns NULL and
> fastrpc_device_open() hits the error path which calls
> fastrpc_channel_ctx_put() on the already-bogus refcount. This
> triggers the free callback, kfree()-ing the channel context. A
> subsequent IRQ-path fastrpc_rpmsg_callback() then dereferences the
> freed spinlock:
> 
>   Unable to handle kernel paging request at virtual address ffffcd5841510ca0
>   Internal error: Oops: 0000000096000047 [#1] SMP
>   Call trace:
>    queued_spin_lock_slowpath+0x414/0x5e0 (P)
>    _raw_spin_lock_irqsave+0x74/0x90
>    fastrpc_rpmsg_callback+0x4c/0xf0 [fastrpc]
>    qcom_glink_native_rx+0x6c4/0xf10
>    qcom_glink_smem_intr+0x1c/0x38 [qcom_glink_smem]
>   Kernel panic - not syncing: Oops: Fatal exception in interrupt
> 
> Fix by moving all channel context initialisation (kref_init,
> spin_lock_init, INIT_LIST_HEAD, idr_init, dev_set_drvdata) and
> fastrpc_cb_devices_create() before fastrpc_device_register(), ensuring
> the struct is fully ready before the device node is visible to
> userspace.
> 
> Add err_depopulate label in the error path since
> fastrpc_cb_devices_create() now runs before misc_register(); a
> fastrpc_device_register() failure must call fastrpc_cb_devices_destroy()
> to clean up the compute-cb children before freeing data.
> 
> Fixes: 3abe3ab3cdab ("misc: fastrpc: add secure domain support")
> Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
Cc: stable for both the fix patches.> ---
>  drivers/misc/fastrpc.c | 43 +++++++++++++++++++++++--------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7ab1cbbf19bb..93d07d28a0f1 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2614,6 +2614,23 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	data->poll_mode_supported = soc_data->poll_mode_supported ||
>  		of_machine_get_match(fastrpc_poll_supported_machines);
>  
> +	kref_init(&data->refcount);
> +	atomic_set(&data->ctx_seq, 0);
> +
> +	rdev->dma_mask = &data->dma_mask;
> +	dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
> +	INIT_LIST_HEAD(&data->users);
> +	INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
> +	spin_lock_init(&data->lock);
> +	idr_init(&data->ctx_idr);
> +	data->domain_id = domain_id;
> +	data->rpdev = rpdev;
> +	dev_set_drvdata(&rpdev->dev, data);
> +
> +	err = fastrpc_cb_devices_create(rpdev);
> +	if (err)
> +		goto err_free_data;
> +
>  	switch (domain_id) {
>  	case ADSP_DOMAIN_ID:
>  	case MDSP_DOMAIN_ID:
> @@ -2622,7 +2639,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		data->unsigned_support = false;
>  		err = fastrpc_device_register(rdev, data, secure_dsp, domain);
>  		if (err)
> -			goto err_free_data;
> +			goto err_depopulate;
>  		break;
>  	case CDSP_DOMAIN_ID:
>  	case GDSP_DOMAIN_ID:
> @@ -2630,7 +2647,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		/* Create both device nodes so that we can allow both Signed and Unsigned PD */
>  		err = fastrpc_device_register(rdev, data, true, domain);
>  		if (err)
> -			goto err_free_data;
> +			goto err_depopulate;
>  
>  		err = fastrpc_device_register(rdev, data, false, domain);
>  		if (err)
> @@ -2638,26 +2655,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		break;
>  	default:
>  		err = -EINVAL;
> -		goto err_free_data;
> +		goto err_depopulate;
>  	}
>  
> -	kref_init(&data->refcount);
> -	atomic_set(&data->ctx_seq, 0);
> -
> -	rdev->dma_mask = &data->dma_mask;
> -	dma_set_mask_and_coherent(rdev, DMA_BIT_MASK(32));
> -	INIT_LIST_HEAD(&data->users);
> -	INIT_LIST_HEAD(&data->invoke_interrupted_mmaps);
> -	spin_lock_init(&data->lock);
> -	idr_init(&data->ctx_idr);
> -	data->domain_id = domain_id;
> -	data->rpdev = rpdev;
> -	dev_set_drvdata(&rpdev->dev, data);
> -
> -	err = fastrpc_cb_devices_create(rpdev);
> -	if (err)
> -		goto err_deregister_fdev;
> -
>  	if (data->domain_id == ADSP_DOMAIN_ID && data->sesscount > 0) {
>  		struct fastrpc_session_ctx *last_sess;
>  		struct fastrpc_session_ctx *dup_sess;
> @@ -2682,6 +2682,9 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	if (data->secure_fdevice)
>  		misc_deregister(&data->secure_fdevice->miscdev);
>  
> +err_depopulate:
> +	fastrpc_cb_devices_destroy(rpdev);
In case of CDSP/GDSP, if secure device register is successful and some
application opens device, that could result in cctx ref being taken and
direct kfree(data) might cause problems, can you check that path also?> +
>  err_free_data:
>  	kfree(data);
>  	return err;
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-14  4:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-01 11:01 [PATCH 0/2] fastrpc: fix probe-time races between rpmsg and userspace Vinayak Katoch
2026-10-01 11:01 ` [PATCH 1/2] misc: fastrpc: fix NULL rpdev dereference when session alloc fails Vinayak Katoch
2026-10-14  4:02   ` Ekansh Gupta
2026-10-01 11:01 ` [PATCH 2/2] misc: fastrpc: fix init ordering race in rpmsg probe Vinayak Katoch
2026-10-14  4:10   ` Ekansh Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®