Re: [PATCH v2] media: v4l2-fwnode: Fix subdev owner overwritten in v4l2_async_register_subdev_sensor()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Mirela,

Thank you for the patch.

On Fri, May 22, 2026 at 05:31:20PM +0300, Mirela Rabulea wrote:
> The v4l2 helper v4l2_async_register_subdev_sensor() calls
> v4l2_async_register_subdev(), which is a macro that expands to
> __v4l2_async_register_subdev(sd,THIS_MODULE). Since the macro is expanded
> inside v4l2-fwnode.c, THIS_MODULE resolves to the v4l2-fwnode module
> rather than the sensor driver module that originally set sd->owner. When
> v4l2-fwnode is built-in, THIS_MODULE evaluates to NULL, which then
> overwrites the sensor driver's owner with NULL.
> 
> This causes the problem that the sensor module's reference count is never
> incremented during async registration, so the module can be removed while
> the subdevice is still in use by a notifier (e.g., a CSI-2 receiver
> bridge driver).
> 
> Fix this by renaming v4l2_async_register_subdev_sensor() to
> __v4l2_async_register_subdev_sensor() with an added explicit module
> argument and introducing a wrapper macro:
>     #define v4l2_async_register_subdev_sensor(sd) \
>         __v4l2_async_register_subdev_sensor(sd, THIS_MODULE)
> 
> This ensures the sensor driver module is properly referenced even when
> the sensor driver does not init the owner field before calling
> v4l2_async_register_subdev_sensor() and prevents premature module removal.
> 
> Fixes: aef69d54755d ("media: v4l: fwnode: Add a convenience function for registering sensors")
> Suggested-by: Frank Li <Frank.Li@nxp.com>
> Link: https://lore.kernel.org/linux-media/20240315073125.275501-2-sakari.ailus@linux.intel.com/
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> 
> Changes in v2:
> 	Do not rely on sd->owner set by v4l2_i2c_subdev_init(), introduce v4l2_async_register_subdev_sensor wrapper macro	
> 	Added Suggested-by: Frank Li <Frank.Li@nxp.com>
> 	Added Link: to Sakari's similar commit for v4l2_async_register_subdev macro
> 
> The v1 patch is also valid, as from what I see, all sensor drivers that use
> v4l2_async_register_subdev_sensor also use v4l2_i2c_subdev_init(), which
> sets sd->owner
> 
> 
>  drivers/media/v4l2-core/v4l2-fwnode.c | 6 +++---
>  include/media/v4l2-async.h            | 4 +++-
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 77f3298821b5..62a3a452f788 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -1256,7 +1256,7 @@ v4l2_async_nf_parse_fwnode_sensor(struct device *dev,
>  	return 0;
>  }
>  
> -int v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd)
> +int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *module)
>  {
>  	struct v4l2_async_notifier *notifier;
>  	int ret;
> @@ -1282,7 +1282,7 @@ int v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd)
>  	if (ret < 0)
>  		goto out_cleanup;
>  
> -	ret = v4l2_async_register_subdev(sd);
> +	ret = __v4l2_async_register_subdev(sd, module);
>  	if (ret < 0)
>  		goto out_unregister;
>  
> @@ -1300,7 +1300,7 @@ int v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd)
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(v4l2_async_register_subdev_sensor);
> +EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_sensor);
>  
>  MODULE_DESCRIPTION("V4L2 fwnode binding parsing library");
>  MODULE_LICENSE("GPL");
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index f26c323e9c96..54a2d9620ed5 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -333,8 +333,10 @@ int __v4l2_async_register_subdev(struct v4l2_subdev *sd, struct module *module);
>   * An error is returned if the module is no longer loaded on any attempts
>   * to register it.
>   */
> +#define v4l2_async_register_subdev_sensor(sd) \
> +	__v4l2_async_register_subdev_sensor(sd, THIS_MODULE)
>  int __must_check
> -v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd);
> +__v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *module);
>  
>  /**
>   * v4l2_async_unregister_subdev - unregisters a sub-device to the asynchronous

-- 
Regards,

Laurent Pinchart