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 <[email protected]> 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/[email protected]?part=1
