Re: [PATCH v2] misc: fastrpc: Reduce console noise from expected DSP info, reserved memory and session messages

[sashiko-bot@kernel.org]

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