Re: [PATCH v2] media: rp1-cfe: Fix use-after-free and double initialization on sensor reload

[xiaolei wang <xiaolei.wang@windriver.com>]

Hi,

Gentle ping on this patch. It fixes use-after-free and double
initialization issues when sensor drivers are reloaded.

Any feedback would be appreciated.

Thanks,
Xiaolei

On 2/12/26 16:55, Xiaolei Wang wrote:
> When a sensor driver is unloaded and reloaded (e.g., rmmod/insmod
> ov5647), the cfe_async_complete callback is invoked again, causing
> multiple issues:
>
> 1. KASAN use-after-free: The callback accesses the sensor subdevice
>     that was freed during driver removal, triggering a use-after-free
>     error
> 2. Double initialization: video_register_device() attempts to
>     re-initialize the video_device kobject that is still registered,
>     causing "tried to init an initialized object" warnings
>
> Fix this by:
> - Moving video node registration from cfe_async_complete() to
>    cfe_probe(), ensuring video devices are registered only once
>    during driver initialization
> - Implementing cfe_async_unbind() callback to clear the sensor
>    pointer when the subdevice is unbound, preventing access to
>    freed memory
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---
>
> Changes in V2:
>    Register video nodes at probe time. I've also updated the commit log; some
>    descriptions in version V1 were inaccurate. This patch resolves the following issues.
>
>    1. KASAN use-after-free: The callback accesses the sensor subdevice
>       that was freed during driver removal, triggering a use-after-free
>       error
>
>       KASAN: slab-use-after-free in cfe_async_complete+0x3a8/0x470 [rp1_cfe]
>       Read of size 2 at addr ffff0001182fa0b8 by task insmod/742
>
>       Call trace:
>        cfe_async_complete+0x3a8/0x470 [rp1_cfe]
>        v4l2_async_nf_try_complete+0xec/0x140 [v4l2_async]
>        __v4l2_async_register_subdev+0x13c/0x3d0 [v4l2_async]
>        v4l2_async_register_subdev_sensor+0x144/0x278 [v4l2_fwnode]
>        ov5647_probe+0xa28/0xd80 [ov5647]
>
>        Allocated by task 220:
>        ov5647_probe+0xc4/0xd80 [ov5647]
>
>        Freed by task 737:
>        i2c_device_remove+0x88/0x190
>        ov5647_driver_exit+0x18/0x48 [ov5647]
>
>    2. Double initialization: video_register_device() attempts to
>       re-initialize the video_device kobject that is still registered,
>       causing "tried to init an initialized object" warnings
>
> Link to V1: https://patchwork.linuxtv.org/project/linux-media/patch/20260211034501.1815035-1-xiaolei.wang@windriver.com/
>
>   .../media/platform/raspberrypi/rp1-cfe/cfe.c  | 39 ++++++++++++-------
>   1 file changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c b/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c
> index 62dca76b468d..c6ba71d1f0d7 100644
> --- a/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c
> +++ b/drivers/media/platform/raspberrypi/rp1-cfe/cfe.c
> @@ -2151,31 +2151,19 @@ static int cfe_probe_complete(struct cfe_device *cfe)
>   
>   	cfe->v4l2_dev.notify = cfe_notify;
>   
> -	for (unsigned int i = 0; i < NUM_NODES; i++) {
> -		ret = cfe_register_node(cfe, i);
> -		if (ret) {
> -			cfe_err(cfe, "Unable to register video node %u.\n", i);
> -			goto unregister;
> -		}
> -	}
> -
>   	ret = cfe_link_node_pads(cfe);
>   	if (ret) {
>   		cfe_err(cfe, "Unable to link node pads.\n");
> -		goto unregister;
> +		return ret;
>   	}
>   
>   	ret = v4l2_device_register_subdev_nodes(&cfe->v4l2_dev);
>   	if (ret) {
>   		cfe_err(cfe, "Unable to register subdev nodes.\n");
> -		goto unregister;
> +		return ret;
>   	}
>   
>   	return 0;
> -
> -unregister:
> -	cfe_unregister_nodes(cfe);
> -	return ret;
>   }
>   
>   static int cfe_async_bound(struct v4l2_async_notifier *notifier,
> @@ -2204,8 +2192,19 @@ static int cfe_async_complete(struct v4l2_async_notifier *notifier)
>   	return cfe_probe_complete(cfe);
>   }
>   
> +static void cfe_async_unbind(struct v4l2_async_notifier *notifier,
> +			     struct v4l2_subdev *subdev,
> +			     struct v4l2_async_connection *asd)
> +{
> +	struct cfe_device *cfe = to_cfe_device(notifier->v4l2_dev);
> +
> +	cfe->source_sd = NULL;
> +	cfe_info(cfe, "Unbinding subdev %s\n", subdev->name);
> +}
> +
>   static const struct v4l2_async_notifier_operations cfe_async_ops = {
>   	.bound = cfe_async_bound,
> +	.unbind = cfe_async_unbind,
>   	.complete = cfe_async_complete,
>   };
>   
> @@ -2243,6 +2242,14 @@ static int cfe_register_async_nf(struct cfe_device *cfe)
>   	cfe->csi2.dphy.max_lanes = ep.bus.mipi_csi2.num_data_lanes;
>   	cfe->csi2.bus_flags = ep.bus.mipi_csi2.flags;
>   
> +	for (unsigned int i = 0; i < NUM_NODES; i++) {
> +		ret = cfe_register_node(cfe, i);
> +		if (ret) {
> +			cfe_err(cfe, "Unable to register video node %u.\n", i);
> +			goto err_unregister;
> +		}
> +	}
> +
>   	/* Initialize and register the async notifier. */
>   	v4l2_async_nf_init(&cfe->notifier, &cfe->v4l2_dev);
>   	cfe->notifier.ops = &cfe_async_ops;
> @@ -2252,7 +2259,7 @@ static int cfe_register_async_nf(struct cfe_device *cfe)
>   	if (IS_ERR(asd)) {
>   		ret = PTR_ERR(asd);
>   		cfe_err(cfe, "Error adding subdevice: %d\n", ret);
> -		goto err_put_local_fwnode;
> +		goto err_unregister;
>   	}
>   
>   	ret = v4l2_async_nf_register(&cfe->notifier);
> @@ -2267,6 +2274,8 @@ static int cfe_register_async_nf(struct cfe_device *cfe)
>   
>   err_nf_cleanup:
>   	v4l2_async_nf_cleanup(&cfe->notifier);
> +err_unregister:
> +	cfe_unregister_nodes(cfe);
>   err_put_local_fwnode:
>   	fwnode_handle_put(local_ep_fwnode);
>