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

New issues:
- [High] Child context bank devices are never attached to their IOMMU domains, 
breaking DMA mapping and causing SMMU faults.

Pre-existing issues:
- [High] Unconditional `kfree(data)` on the probe error path causes a 
Use-After-Free if userspace opened the device.
--

commit e928c95aa5ca7ef57a0d4943351ae2891fcf2b1c
Author: Vinayak Katoch <[email protected]>

misc: fastrpc: iterate CB nodes manually instead of of_platform_populate

This commit replaces of_platform_populate() with manual iteration over child
device tree nodes to initialize qcom,fastrpc-compute-cb devices synchronously.
It aims to ensure all context banks are fully initialized before
fastrpc_rpmsg_probe() returns by directly calling fastrpc_cb_init().

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index d4fac2caca867..a153107d00853 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2363,7 +2363,7 @@ static const struct file_operations fastrpc_fops = {
>       .compat_ioctl = fastrpc_device_ioctl,
>  };
>  
> -static int fastrpc_cb_probe(struct platform_device *pdev)
> +static int fastrpc_cb_init(struct platform_device *pdev)
>  {
>       struct fastrpc_channel_ctx *cctx;
>       struct fastrpc_session_ctx *sess;

[ ... ]

> @@ -2642,12 +2610,29 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device 
> *rpdev)
>       data->rpdev = rpdev;
>       dev_set_drvdata(&rpdev->dev, data);
>  
> -     err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
> -     if (err)
> -             goto err_deregister_fdev;
> +     of_node_set_flag(rdev->of_node, OF_POPULATED_BUS);
> +
> +     for_each_available_child_of_node_scoped(rdev->of_node, np) {
> +             struct platform_device *pdev;
> +
> +             if (!of_device_is_compatible(np, "qcom,fastrpc-compute-cb"))
> +                     continue;
> +
> +             pdev = of_platform_device_create(np, NULL, rdev);
> +             if (!pdev) {
> +                     err = -EINVAL;
> +                     goto err_depopulate;
> +             }
> +
> +             err = fastrpc_cb_init(pdev);

[Severity: High]
By replacing driver core matching with manual of_platform_device_create()
followed by fastrpc_cb_init() in fastrpc_rpmsg_probe(), does this child
device skip the driver core's platform probe path?

It appears this manual initialization skips IOMMU and DMA configuration
logic, such as of_dma_configure(), which normally runs during probe.

Since these context bank devices are no longer bound to a platform driver,
they are never attached to their SMMU domains. Could this result in SMMU
translation faults and a failure of DSP context bank DMA operations when a
DSP invocation attempts DMA?

> +             if (err)
> +                     goto err_depopulate;
> +     }
>  
>       return 0;
>  
> +err_depopulate:
> +     of_platform_depopulate(rdev);
>  err_deregister_fdev:
>       if (data->fdevice)
>               misc_deregister(&data->fdevice->miscdev);

[Severity: High]
This is a pre-existing issue, but if fastrpc_cb_init() fails, this error path
jumps to err_depopulate and eventually reaches err_free_data in
fastrpc_rpmsg_probe():

err_free_data:
        kfree(data);
        return err;

Since fastrpc_device_register() makes the misc device available to userspace
earlier in the probe, what happens if userspace has already called open()
and incremented the data reference count via fastrpc_channel_ctx_get()?

Does this unconditional kfree(data) ignore active references held by open
file descriptors, leading to a use-after-free condition if userspace later
performs an ioctl() on the still-open file descriptor?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=3

Reply via email to