* [PATCH] v4l: Reset subdev v4l2_dev field to NULL if registration fails
@ 2012-11-25 0:41 Laurent Pinchart
2012-11-27 22:54 ` Sylwester Nawrocki
0 siblings, 1 reply; 3+ messages in thread
From: Laurent Pinchart @ 2012-11-25 0:41 UTC (permalink / raw)
To: linux-media
When subdev registration fails the subdev v4l2_dev field is left to a
non-NULL value. Later calls to v4l2_device_unregister_subdev() will
consider the subdev as registered and will module_put() the subdev
module without any matching module_get().
Fix this by setting the subdev v4l2_dev field to NULL in
v4l2_device_register_subdev() when the function fails.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/media/v4l2-core/v4l2-device.c | 30 ++++++++++++++----------------
1 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 513969f..98a7f5e 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -159,31 +159,21 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
sd->v4l2_dev = v4l2_dev;
if (sd->internal_ops && sd->internal_ops->registered) {
err = sd->internal_ops->registered(sd);
- if (err) {
- module_put(sd->owner);
- return err;
- }
+ if (err)
+ goto error_module;
}
/* This just returns 0 if either of the two args is NULL */
err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, sd->ctrl_handler, NULL);
- if (err) {
- if (sd->internal_ops && sd->internal_ops->unregistered)
- sd->internal_ops->unregistered(sd);
- module_put(sd->owner);
- return err;
- }
+ if (err)
+ goto error_unregister;
#if defined(CONFIG_MEDIA_CONTROLLER)
/* Register the entity. */
if (v4l2_dev->mdev) {
err = media_device_register_entity(v4l2_dev->mdev, entity);
- if (err < 0) {
- if (sd->internal_ops && sd->internal_ops->unregistered)
- sd->internal_ops->unregistered(sd);
- module_put(sd->owner);
- return err;
- }
+ if (err < 0)
+ goto error_unregister;
}
#endif
@@ -192,6 +182,14 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
spin_unlock(&v4l2_dev->lock);
return 0;
+
+error_unregister:
+ if (sd->internal_ops && sd->internal_ops->unregistered)
+ sd->internal_ops->unregistered(sd);
+error_module:
+ module_put(sd->owner);
+ sd->v4l2_dev = NULL;
+ return err;
}
EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
--
1.7.8.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] v4l: Reset subdev v4l2_dev field to NULL if registration fails
2012-11-25 0:41 [PATCH] v4l: Reset subdev v4l2_dev field to NULL if registration fails Laurent Pinchart
@ 2012-11-27 22:54 ` Sylwester Nawrocki
2012-11-28 13:36 ` Laurent Pinchart
0 siblings, 1 reply; 3+ messages in thread
From: Sylwester Nawrocki @ 2012-11-27 22:54 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media
Hi Laurent,
On 11/25/2012 01:41 AM, Laurent Pinchart wrote:
> When subdev registration fails the subdev v4l2_dev field is left to a
> non-NULL value. Later calls to v4l2_device_unregister_subdev() will
> consider the subdev as registered and will module_put() the subdev
> module without any matching module_get().
>
> Fix this by setting the subdev v4l2_dev field to NULL in
> v4l2_device_register_subdev() when the function fails.
>
> Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
I'm just wondering whether including this patch in stable kernel releases
could potentially break anything.
> ---
> drivers/media/v4l2-core/v4l2-device.c | 30 ++++++++++++++----------------
> 1 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
> index 513969f..98a7f5e 100644
> --- a/drivers/media/v4l2-core/v4l2-device.c
> +++ b/drivers/media/v4l2-core/v4l2-device.c
> @@ -159,31 +159,21 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
> sd->v4l2_dev = v4l2_dev;
> if (sd->internal_ops&& sd->internal_ops->registered) {
> err = sd->internal_ops->registered(sd);
> - if (err) {
> - module_put(sd->owner);
> - return err;
> - }
> + if (err)
> + goto error_module;
> }
>
> /* This just returns 0 if either of the two args is NULL */
> err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, sd->ctrl_handler, NULL);
> - if (err) {
> - if (sd->internal_ops&& sd->internal_ops->unregistered)
> - sd->internal_ops->unregistered(sd);
> - module_put(sd->owner);
> - return err;
> - }
> + if (err)
> + goto error_unregister;
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> /* Register the entity. */
> if (v4l2_dev->mdev) {
> err = media_device_register_entity(v4l2_dev->mdev, entity);
> - if (err< 0) {
> - if (sd->internal_ops&& sd->internal_ops->unregistered)
> - sd->internal_ops->unregistered(sd);
> - module_put(sd->owner);
> - return err;
> - }
> + if (err< 0)
> + goto error_unregister;
> }
> #endif
>
> @@ -192,6 +182,14 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
> spin_unlock(&v4l2_dev->lock);
>
> return 0;
> +
> +error_unregister:
> + if (sd->internal_ops&& sd->internal_ops->unregistered)
> + sd->internal_ops->unregistered(sd);
> +error_module:
> + module_put(sd->owner);
> + sd->v4l2_dev = NULL;
> + return err;
> }
> EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] v4l: Reset subdev v4l2_dev field to NULL if registration fails
2012-11-27 22:54 ` Sylwester Nawrocki
@ 2012-11-28 13:36 ` Laurent Pinchart
0 siblings, 0 replies; 3+ messages in thread
From: Laurent Pinchart @ 2012-11-28 13:36 UTC (permalink / raw)
To: Sylwester Nawrocki; +Cc: linux-media
Hi Sylwester,
On Tuesday 27 November 2012 23:54:40 Sylwester Nawrocki wrote:
> On 11/25/2012 01:41 AM, Laurent Pinchart wrote:
> > When subdev registration fails the subdev v4l2_dev field is left to a
> > non-NULL value. Later calls to v4l2_device_unregister_subdev() will
> > consider the subdev as registered and will module_put() the subdev
> > module without any matching module_get().
> >
> > Fix this by setting the subdev v4l2_dev field to NULL in
> > v4l2_device_register_subdev() when the function fails.
> >
> > Signed-off-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
>
> Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Thank you.
> I'm just wondering whether including this patch in stable kernel releases
> could potentially break anything.
I don't think it would, the patch only touches error paths, and clearly fixes
a bug. So
Cc: stable@vger.kernel.org
looks like a good idea.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-11-28 13:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-25 0:41 [PATCH] v4l: Reset subdev v4l2_dev field to NULL if registration fails Laurent Pinchart
2012-11-27 22:54 ` Sylwester Nawrocki
2012-11-28 13:36 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.