[v1] media: v4l2-fwnode: Fix subdev owner overwritten in v4l2_async_register_subdev_sensor()

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

Posted by Mirela Rabulea 3 days, 7 hours ago

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 calling __v4l2_async_register_subdev() directly with
sd->owner, which preserves the module owner that the sensor driver set
during probe via v4l2_i2c_subdev_init() or direct assignment.

Fixes: aef69d54755d ("media: v4l: fwnode: Add a convenience function for registering sensors")

Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 77f3298821b5..57284b7adddf 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -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, sd->owner);
 	if (ret < 0)
 		goto out_unregister;
 
-- 
2.43.0

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

Posted by Frank Li 3 days ago

On Thu, May 21, 2026 at 03:26:45PM +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 is problem. but this patch can't resolve problem 100% because
many sensor driver have not init sd->owner before call
v4l2_async_register_subdev_sensor()

suggest solution like what v4l2_async_register_subdev() did

rename v4l2_async_register_subdev_sensor() to
__v4l2_async_register_subdev_sensor(truct v4l2_subdev *sd, module *this)

#define v4l2_async_register_subdev_sensor(sd)
	__v4l2_async_register_subdev_sensor(sd, THIS_MODULE)

Frank

>
> 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 calling __v4l2_async_register_subdev() directly with
> sd->owner, which preserves the module owner that the sensor driver set
> during probe via v4l2_i2c_subdev_init() or direct assignment.
>
> Fixes: aef69d54755d ("media: v4l: fwnode: Add a convenience function for registering sensors")
>
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 77f3298821b5..57284b7adddf 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -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, sd->owner);
>  	if (ret < 0)
>  		goto out_unregister;
>
> --
> 2.43.0
>

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

Posted by Mirela Rabulea 2 days, 5 hours ago

Hi Frank,

On 5/21/26 21:45, Frank Li wrote:
> On Thu, May 21, 2026 at 03:26:45PM +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 is problem. but this patch can't resolve problem 100% because
> many sensor driver have not init sd->owner before call
> v4l2_async_register_subdev_sensor()
Thanks for the suggestion. From what I see, all sensor drivers that use 
v4l2_async_register_subdev_sensor() also use v4l2_i2c_subdev_init(), which

sets sd->owner. However, since your proposal looks more robust and there 
is already a similar solution accepted for v4l2_async_register_subdev(), 
I will send a v2 with that approach.

Thanks,

Mirela

>
> suggest solution like what v4l2_async_register_subdev() did
>
> rename v4l2_async_register_subdev_sensor() to
> __v4l2_async_register_subdev_sensor(truct v4l2_subdev *sd, module *this)
>
> #define v4l2_async_register_subdev_sensor(sd)
> 	__v4l2_async_register_subdev_sensor(sd, THIS_MODULE)
>
> Frank
>
>> 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 calling __v4l2_async_register_subdev() directly with
>> sd->owner, which preserves the module owner that the sensor driver set
>> during probe via v4l2_i2c_subdev_init() or direct assignment.
>>
>> Fixes: aef69d54755d ("media: v4l: fwnode: Add a convenience function for registering sensors")
>>
>> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
>> ---
>>   drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
>> index 77f3298821b5..57284b7adddf 100644
>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>> @@ -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, sd->owner);
>>   	if (ret < 0)
>>   		goto out_unregister;
>>
>> --
>> 2.43.0
>>

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

Posted by Frank Li 2 days, 1 hour ago

On Fri, May 22, 2026 at 05:30:58PM +0300, Mirela Rabulea wrote:
> Hi Frank,
>
> On 5/21/26 21:45, Frank Li wrote:
> > On Thu, May 21, 2026 at 03:26:45PM +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 is problem. but this patch can't resolve problem 100% because
> > many sensor driver have not init sd->owner before call
> > v4l2_async_register_subdev_sensor()
> Thanks for the suggestion. From what I see, all sensor drivers that use
> v4l2_async_register_subdev_sensor() also use v4l2_i2c_subdev_init(), which
>
> sets sd->owner. However, since your proposal looks more robust and there is
> already a similar solution accepted for v4l2_async_register_subdev(), I will
> send a v2 with that approach.

Okay, that's also make sense. let's wait for maintainers' opinion.

Frank
>
> Thanks,
>
> Mirela
>
> >
> > suggest solution like what v4l2_async_register_subdev() did
> >
> > rename v4l2_async_register_subdev_sensor() to
> > __v4l2_async_register_subdev_sensor(truct v4l2_subdev *sd, module *this)
> >
> > #define v4l2_async_register_subdev_sensor(sd)
> > 	__v4l2_async_register_subdev_sensor(sd, THIS_MODULE)
> >
> > Frank
> >
> > > 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 calling __v4l2_async_register_subdev() directly with
> > > sd->owner, which preserves the module owner that the sensor driver set
> > > during probe via v4l2_i2c_subdev_init() or direct assignment.
> > >
> > > Fixes: aef69d54755d ("media: v4l: fwnode: Add a convenience function for registering sensors")
> > >
> > > Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> > > ---
> > >   drivers/media/v4l2-core/v4l2-fwnode.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > index 77f3298821b5..57284b7adddf 100644
> > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > > @@ -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, sd->owner);
> > >   	if (ret < 0)
> > >   		goto out_unregister;
> > >
> > > --
> > > 2.43.0
> > >