Re: [PATCH v1 1/3] misc: fastrpc: initialise channel refcount before exposing the misc device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] misc: fastrpc: uninitialized spin_lock access due to premature registration
- [High] misc: fastrpc: UAF on probe error path due to direct kfree
- [High] misc: fastrpc: UAF in fastrpc_rpmsg_probe error path due to raw kfree
--

commit 7fb377963b47742ec8445ae7a99aea90b1ae5375
Author: Jianping Li <jianping.li@oss.qualcomm.com>

misc: fastrpc: initialise channel refcount before exposing the misc device

This patch moves kref_init() before fastrpc_device_register() to fix a
refcount warning when userspace opens the misc device concurrently during
probe.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index af18ff1992eed..05b2e7e4ad3b7 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2599,6 +2599,7 @@ 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);
>  	switch (domain_id) {
>  	case ADSP_DOMAIN_ID:
>  	case MDSP_DOMAIN_ID:

[ ... ]

> @@ -2626,7 +2627,6 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  		goto err_free_data;
>  	}
>  
> -	kref_init(&data->refcount);
>  	atomic_set(&data->ctx_seq, 0);
>  
>  	rdev->dma_mask = &data->dma_mask;

[Severity: High]
Does exposing the device to userspace before initializing the spinlock result
in a fatal crash if the device is opened immediately?

In fastrpc_rpmsg_probe(), spin_lock_init(&data->lock) is called further down,
after the switch statement. If userspace opens the device immediately after
fastrpc_device_register(), the open path calls fastrpc_session_alloc(), which
attempts to acquire the uninitialized lock:

fastrpc_device_open()
   fastrpc_session_alloc()
      spin_lock_irqsave(&cctx->lock, flags);

[Severity: High]
Does the probe error path cause a use-after-free if userspace has successfully
opened the device?

Since kref_init() now executes before the device registration, an early open
can successfully acquire a reference via fastrpc_channel_ctx_get().

If a subsequent probe step fails (such as the second fastrpc_device_register()
call for CDSP/GDSP, or of_platform_populate()), the probe error path directly
frees the channel context:

err_free_data:
	kfree(data);
	return err;

This unconditional kfree() bypasses the kref lifecycle. When userspace later
closes the file descriptor, fastrpc_device_release() calls
fastrpc_channel_ctx_put(), which will dereference the freed memory.

Should the error path use fastrpc_channel_ctx_put() once the kref has been
initialized, instead of calling kfree() directly?

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