All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] misc: fastrpc: Reduce console noise from expected DSP info, reserved memory and session messages
@ 2026-09-07  7:03 Jianping Li
  2026-09-07  7:14 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Jianping Li @ 2026-09-07  7:03 UTC (permalink / raw)
  To: Srinivas Kandagatla, Ekansh Gupta
  Cc: Jianping Li, Arnd Bergmann, Greg Kroah-Hartman, linux-arm-msm,
	dri-devel, linux-kernel, Dmitry Baryshkov, quic_chennak

On some platforms the DSP firmware does not support the fastrpc
capability/attribute query. The DSP side returns an error (err: -1)
while handling the request, so fastrpc_get_info_from_dsp() does not
return DSP_UNSUPPORTED_API but a generic error, and the "else if (err)"
branch in fastrpc_get_info_from_kernel() is taken.

cctx->valid_attributes is therefore never set and every open()/query
re-prints the same "dsp information is incorrect" line. This is not
tied to boot timing: it can be reproduced at any time by manually
starting adsprpcd. On RC builds with metadata flashing enabled several
clients come up and flood the console.

As this is an expected, platform-specific and non-fatal condition,
downgrade the message to dev_dbg. Similarly, the absence of reserved
DMA memory is a valid configuration and does not indicate an error,
so downgrade that message to dev_dbg as well. Also rate-limit the
"No session available" message with dev_err_ratelimited(): it reflects
a genuine failure (open() returns -EBUSY) so it should still be visible,
but must not flood dmesg when the session pool is exhausted.

No functional change intended.

Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
---
Patch [v1]: https://lore.kernel.org/all/20260514062825.50172-1-jianping.li@oss.qualcomm.com/

Changes in v2:
  - Add "misc: fastrpc:" subject prefix.
  - Reword commit message: drop "temporarily", describe the actual scenario
	and state the errno returned by the DSP.
  - Rate-limit "No session available" with dev_err_ratelimited().
---
 drivers/misc/fastrpc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index d4fac2caca86..1f9531a534b9 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1783,7 +1783,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
 
 	fl->sctx = fastrpc_session_alloc(fl);
 	if (!fl->sctx) {
-		dev_err(&cctx->rpdev->dev, "No session available\n");
+		dev_err_ratelimited(&cctx->rpdev->dev, "No session available\n");
 		mutex_destroy(&fl->mutex);
 		kfree(fl);
 		fastrpc_channel_ctx_put(cctx);
@@ -1937,7 +1937,7 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
 		kfree(dsp_attributes);
 		return -EOPNOTSUPP;
 	} else if (err) {
-		dev_err(&cctx->rpdev->dev, "Error: dsp information is incorrect err: %d\n", err);
+		dev_dbg(&cctx->rpdev->dev, "Error: dsp information is incorrect err: %d\n", err);
 		kfree(dsp_attributes);
 		return err;
 	}
@@ -2559,7 +2559,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 	}
 
 	if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0))
-		dev_info(rdev, "no reserved DMA memory for FASTRPC\n");
+		dev_dbg(rdev, "no reserved DMA memory for FASTRPC\n");
 
 	vmcount = of_property_read_variable_u32_array(rdev->of_node,
 				"qcom,vmids", &vmids[0], 0, FASTRPC_MAX_VMIDS);
-- 
2.43.0


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

* Re: [PATCH v2] misc: fastrpc: Reduce console noise from expected DSP info, reserved memory and session messages
  2026-09-07  7:03 [PATCH v2] misc: fastrpc: Reduce console noise from expected DSP info, reserved memory and session messages Jianping Li
@ 2026-09-07  7:14 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-07  7:14 UTC (permalink / raw)
  To: Jianping Li; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Character device is registered and exposed to user-space before critical context fields (spinlocks, krefs, lists, idr) are initialized, enabling severe race conditions and NULL pointer dereferences on concurrent open().
- [High] Use-After-Free on `fastrpc_rpmsg_probe` error paths after device registration.
--

commit 449b11a15887fd75279efb078924d324d2fb8fb1
Author: Jianping Li <jianping.li@oss.qualcomm.com>
Subject: misc: fastrpc: Reduce console noise from expected DSP info, reserved memory and session messages

The commit downgrades expected error messages from the DSP and reserved DMA
memory to dev_dbg, and rate-limits the session availability error to reduce
console flooding on certain platforms.

Link: https://lore.kernel.org/all/20260514062825.50172-1-jianping.li@oss.qualcomm.com/

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 90fd669636ec1..f40356c950519 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c

[ ... ]

> @@ -2563,7 +2563,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	}
>  
>  	if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0))
> -		dev_info(rdev, "no reserved DMA memory for FASTRPC\n");
> +		dev_dbg(rdev, "no reserved DMA memory for FASTRPC\n");
>  
>  	vmcount = of_property_read_variable_u32_array(rdev->of_node,
>  				"qcom,vmids", &vmids[0], 0, FASTRPC_MAX_VMIDS);

[Severity: High]
This isn't a bug introduced by this patch, but could the initialization in
fastrpc_rpmsg_probe() lead to a race condition with concurrent opens?

Looking at fastrpc_rpmsg_probe(), we register the device before initializing
important context fields:

	err = fastrpc_device_register(rdev, data, secure_dsp, domain);
	if (err)
		goto err_free_data;
	...
	kref_init(&data->refcount);
	atomic_set(&data->ctx_seq, 0);
	...
	INIT_LIST_HEAD(&data->users);
	spin_lock_init(&data->lock);

If a user-space process opens the character device immediately after
fastrpc_device_register() completes, fastrpc_device_open() will be called.

Since data->refcount and data->users are uninitialized, won't this cause a
null pointer dereference when it calls fastrpc_channel_ctx_get() or attempts
to lock data->lock and add to the zeroed data->users list?

[Severity: High]
This is also a pre-existing issue, but is there a risk of a use-after-free
in the error handling paths of fastrpc_rpmsg_probe()?

If an error occurs after the device is registered (for example, if a
subsequent fastrpc_device_register() fails or if of_platform_populate()
fails), the code branches to the teardown paths:

	err = fastrpc_device_register(rdev, data, false, domain);
	if (err)
		goto err_deregister_fdev;
...
err_deregister_fdev:
	if (data->fdevice)
		misc_deregister(&data->fdevice->miscdev);
	if (data->secure_fdevice)
		misc_deregister(&data->secure_fdevice->miscdev);

err_free_data:
	kfree(data);
	return err;

Since misc_deregister() does not revoke active file descriptors, what happens
if a concurrent open() has already acquired a reference to the device before
we call misc_deregister()?

Will the unconditional kfree(data) result in a use-after-free later when the
user closes the file and fastrpc_device_release() accesses the freed data?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907070356.509-1-jianping.li@oss.qualcomm.com?part=1

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

end of thread, other threads:[~2026-09-07  7:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07  7:03 [PATCH v2] misc: fastrpc: Reduce console noise from expected DSP info, reserved memory and session messages Jianping Li
2026-09-07  7:14 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.