Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues: - [High] Leaked and dangling session slots on `fastrpc_cb_probe` error path. - [High] Decrementing `sesscount` in `fastrpc_cb_remove` corrupts the session array. - [High] Missing `of_reserved_mem_device_release` in probe error paths and remove function. -- commit 187d5a897b2d7759c52e28a8af47ac213c1e4782 Author: Pengpeng Hou <[email protected]> misc: fastrpc: add missing MODULE_DEVICE_TABLE() The driver has a match table for the of bus wired into its driver structure, but the table is not exported with MODULE_DEVICE_TABLE(). Add the missing MODULE_DEVICE_TABLE() entry so module alias information is generated for automatic module loading. > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index f3a49384586d1..a5ccdd3eeea11 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -2292,6 +2292,7 @@ static const struct of_device_id fastrpc_match_table[] > = { > { .compatible = "qcom,fastrpc-compute-cb", }, > {} > }; > +MODULE_DEVICE_TABLE(of, fastrpc_match_table); > > static struct platform_driver fastrpc_cb_driver = { > .probe = fastrpc_cb_probe, [Severity: High] This is a pre-existing issue, but does fastrpc_cb_probe() leak session slots on its error path? drivers/misc/fastrpc.c:fastrpc_cb_probe() { ... if (sessions > 0) { struct fastrpc_session_ctx *dup_sess; for (i = 1; i < sessions; i++) { if (cctx->sesscount >= FASTRPC_MAX_SESSIONS) break; dup_sess = &cctx->session[cctx->sesscount++]; memcpy(dup_sess, sess, sizeof(*dup_sess)); } } spin_unlock_irqrestore(&cctx->lock, flags); rc = dma_set_mask(dev, DMA_BIT_MASK(dma_bits)); if (rc) { dev_err(dev, "%u-bit DMA enable failed\n", dma_bits); return rc; } ... } If dma_set_mask() fails, it returns directly without invalidating the appended session(s) or rewinding sesscount. Since sesscount was incremented, fastrpc_session_alloc() might hand out this broken session slot to a userspace context, which will then attempt to use the uninitialized device for DMA operations. > .remove = fastrpc_cb_remove, [Severity: High] This isn't a bug introduced by this patch, but could fastrpc_cb_remove() corrupt the session array when decrementing the total count? drivers/misc/fastrpc.c:fastrpc_cb_remove() { ... spin_lock_irqsave(&cctx->lock, flags); for (i = 0; i < FASTRPC_MAX_SESSIONS; i++) { if (cctx->session[i].sid == sess->sid) { cctx->session[i].valid = false; cctx->sesscount--; } } spin_unlock_irqrestore(&cctx->lock, flags); ... } Decrementing the total count without closing the gap in the array breaks the tracking logic. Because fastrpc_session_alloc() iterates only up to sesscount, any valid sessions beyond the new sesscount become immediately unreachable. A subsequent probe will blindly append at the new sesscount, potentially overwriting a session that is actively in use by userspace. > .driver = { > .name = "qcom,fastrpc-cb", > .of_match_table = fastrpc_match_table, > .suppress_bind_attrs = true, > }, > }; [Severity: High] This is also a pre-existing issue, but is there a missing call to of_reserved_mem_device_release() in the rpmsg probe error paths and remove function? drivers/misc/fastrpc.c:fastrpc_rpmsg_probe() { ... if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0)) dev_info(rdev, "no reserved DMA memory for FASTRPC\n"); ... } drivers/misc/fastrpc.c:fastrpc_rpmsg_remove() { ... of_platform_depopulate(&rpdev->dev); fastrpc_channel_ctx_put(cctx); } The reserved memory is bound in fastrpc_rpmsg_probe() but never released during teardown in fastrpc_rpmsg_remove() or error rollback. Repeated unbind and bind sequences could leak DMA reserved memory mappings. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
