From 6d8acd02c4c6a8f917eefac1de2e035521ca119d Mon Sep 17 00:00:00 2001 From: Alexander Stein Date: Thu, 7 Mar 2024 15:24:51 +0100 Subject: media: v4l: async: Fix notifier list entry init struct v4l2_async_notifier has several list_head members, but only waiting_list and done_list are initialized. notifier_entry was kept 'zeroed' leading to an uninitialized list_head. This results in a NULL-pointer dereference if csi2_async_register() fails, e.g. node for remote endpoint is disabled, and returns -ENOTCONN. The following calls to v4l2_async_nf_unregister() results in a NULL pointer dereference. Add the missing list head initializer. Fixes: b8ec754ae4c5 ("media: v4l: async: Set v4l2_device and subdev in async notifier init") Cc: # for 6.6 and later Signed-off-by: Alexander Stein Signed-off-by: Sakari Ailus Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-async.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/media/v4l2-core/v4l2-async.c') diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 3ec323bd528b..6a7dcf43d712 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -563,6 +563,7 @@ void v4l2_async_nf_init(struct v4l2_async_notifier *notifier, { INIT_LIST_HEAD(¬ifier->waiting_list); INIT_LIST_HEAD(¬ifier->done_list); + INIT_LIST_HEAD(¬ifier->notifier_entry); notifier->v4l2_dev = v4l2_dev; } EXPORT_SYMBOL(v4l2_async_nf_init); @@ -572,6 +573,7 @@ void v4l2_async_subdev_nf_init(struct v4l2_async_notifier *notifier, { INIT_LIST_HEAD(¬ifier->waiting_list); INIT_LIST_HEAD(¬ifier->done_list); + INIT_LIST_HEAD(¬ifier->notifier_entry); notifier->sd = sd; } EXPORT_SYMBOL_GPL(v4l2_async_subdev_nf_init); -- cgit v1.2.3-59-g8ed1b From 46bc0234ad38063ce550ecf135c1a52458f0a804 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Fri, 8 Mar 2024 15:07:45 +0200 Subject: media: v4l: async: Don't set notifier's V4L2 device if registering fails The V4L2 device used to be set when the notifier was registered but this has been moved to the notifier initialisation. Don't touch the V4L2 device if registration fails. Fixes: b8ec754ae4c5 ("media: v4l: async: Set v4l2_device and subdev in async notifier init") Cc: # for 6.6 and later Signed-off-by: Sakari Ailus Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-async.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/media/v4l2-core/v4l2-async.c') diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 6a7dcf43d712..2ff35d5d60f2 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -620,16 +620,10 @@ err_unlock: int v4l2_async_nf_register(struct v4l2_async_notifier *notifier) { - int ret; - if (WARN_ON(!notifier->v4l2_dev == !notifier->sd)) return -EINVAL; - ret = __v4l2_async_nf_register(notifier); - if (ret) - notifier->v4l2_dev = NULL; - - return ret; + return __v4l2_async_nf_register(notifier); } EXPORT_SYMBOL(v4l2_async_nf_register); -- cgit v1.2.3-59-g8ed1b From 9537a8425a7a0222999d5839a0b394b1e8834b4a Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Fri, 8 Mar 2024 15:06:13 +0200 Subject: media: v4l: async: Properly re-initialise notifier entry in unregister The notifier_entry of a notifier is not re-initialised after unregistering the notifier. This leads to dangling pointers being left there so use list_del_init() to return the notifier_entry an empty list. Fixes: b8ec754ae4c5 ("media: v4l: async: Set v4l2_device and subdev in async notifier init") Cc: # for 6.6 and later Signed-off-by: Sakari Ailus Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-async.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/media/v4l2-core/v4l2-async.c') diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 2ff35d5d60f2..4bb073587817 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -635,7 +635,7 @@ __v4l2_async_nf_unregister(struct v4l2_async_notifier *notifier) v4l2_async_nf_unbind_all_subdevs(notifier); - list_del(¬ifier->notifier_entry); + list_del_init(¬ifier->notifier_entry); } void v4l2_async_nf_unregister(struct v4l2_async_notifier *notifier) -- cgit v1.2.3-59-g8ed1b From aa7b148855e8265c4f72ae0305842efdda72680b Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Wed, 13 Mar 2024 14:03:19 +0000 Subject: media: v4l: Set sub-device's owner field to the caller's module Set a sub-device's owner field to the caller's module, provided as an argument to the function. v4l2_device_register_subdev() becomes a macro passing THIS_MODULE to the __v4l2_device_register_subdev() function. Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-async.c | 2 +- drivers/media/v4l2-core/v4l2-device.c | 12 +++++++----- drivers/media/v4l2-core/v4l2-i2c.c | 2 +- drivers/media/v4l2-core/v4l2-spi.c | 2 +- include/media/v4l2-device.h | 7 +++++-- 5 files changed, 15 insertions(+), 10 deletions(-) (limited to 'drivers/media/v4l2-core/v4l2-async.c') diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 4bb073587817..b28f6811a5f4 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -341,7 +341,7 @@ static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, int ret; if (list_empty(&sd->asc_list)) { - ret = v4l2_device_register_subdev(v4l2_dev, sd); + ret = __v4l2_device_register_subdev(v4l2_dev, sd, sd->owner); if (ret < 0) return ret; registered = true; diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c index d2e58ae91f9b..5e537454f5cd 100644 --- a/drivers/media/v4l2-core/v4l2-device.c +++ b/drivers/media/v4l2-core/v4l2-device.c @@ -108,8 +108,8 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev) } EXPORT_SYMBOL_GPL(v4l2_device_unregister); -int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, - struct v4l2_subdev *sd) +int __v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, + struct v4l2_subdev *sd, struct module *module) { int err; @@ -125,9 +125,9 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, * try_module_get() such sub-device owners. */ sd->owner_v4l2_dev = v4l2_dev->dev && v4l2_dev->dev->driver && - sd->owner == v4l2_dev->dev->driver->owner; + module == v4l2_dev->dev->driver->owner; - if (!sd->owner_v4l2_dev && !try_module_get(sd->owner)) + if (!sd->owner_v4l2_dev && !try_module_get(module)) return -ENODEV; sd->v4l2_dev = v4l2_dev; @@ -152,6 +152,8 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, goto error_unregister; } + sd->owner = module; + spin_lock(&v4l2_dev->lock); list_add_tail(&sd->list, &v4l2_dev->subdevs); spin_unlock(&v4l2_dev->lock); @@ -168,7 +170,7 @@ error_module: sd->v4l2_dev = NULL; return err; } -EXPORT_SYMBOL_GPL(v4l2_device_register_subdev); +EXPORT_SYMBOL_GPL(__v4l2_device_register_subdev); static void v4l2_subdev_release(struct v4l2_subdev *sd) { diff --git a/drivers/media/v4l2-core/v4l2-i2c.c b/drivers/media/v4l2-core/v4l2-i2c.c index b4acca75644b..586c46544255 100644 --- a/drivers/media/v4l2-core/v4l2-i2c.c +++ b/drivers/media/v4l2-core/v4l2-i2c.c @@ -100,7 +100,7 @@ struct v4l2_subdev * Register with the v4l2_device which increases the module's * use count as well. */ - if (v4l2_device_register_subdev(v4l2_dev, sd)) + if (__v4l2_device_register_subdev(v4l2_dev, sd, sd->owner)) sd = NULL; /* Decrease the module use count to match the first try_module_get. */ module_put(client->dev.driver->owner); diff --git a/drivers/media/v4l2-core/v4l2-spi.c b/drivers/media/v4l2-core/v4l2-spi.c index a7092c3930d6..1baf8e63f19e 100644 --- a/drivers/media/v4l2-core/v4l2-spi.c +++ b/drivers/media/v4l2-core/v4l2-spi.c @@ -59,7 +59,7 @@ struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev, * Register with the v4l2_device which increases the module's * use count as well. */ - if (v4l2_device_register_subdev(v4l2_dev, sd)) + if (__v4l2_device_register_subdev(v4l2_dev, sd, sd->owner)) sd = NULL; /* Decrease the module use count to match the first try_module_get. */ diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h index f6f111fae33c..dd897a362f36 100644 --- a/include/media/v4l2-device.h +++ b/include/media/v4l2-device.h @@ -156,8 +156,11 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev); * An error is returned if the module is no longer loaded on any attempts * to register it. */ -int __must_check v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, - struct v4l2_subdev *sd); +#define v4l2_device_register_subdev(v4l2_dev, sd) \ + __v4l2_device_register_subdev(v4l2_dev, sd, THIS_MODULE) +int __must_check __v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, + struct v4l2_subdev *sd, + struct module *module); /** * v4l2_device_unregister_subdev - Unregisters a subdev with a v4l2 device. -- cgit v1.2.3-59-g8ed1b From 8a718752f5c339137c5b05e54f116cd26d5a4143 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Wed, 13 Mar 2024 13:57:50 +0000 Subject: media: v4l: async: Set owner for async sub-devices Set the owner field of the async sub-devices by making v4l2_async_register_subdev() a macro and obtaining THIS_MODULE that way. Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-async.c | 8 ++++++-- include/media/v4l2-async.h | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'drivers/media/v4l2-core/v4l2-async.c') diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index b28f6811a5f4..3a22da443a22 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -783,7 +783,7 @@ v4l2_async_connection_unique(struct v4l2_subdev *sd) } EXPORT_SYMBOL_GPL(v4l2_async_connection_unique); -int v4l2_async_register_subdev(struct v4l2_subdev *sd) +int __v4l2_async_register_subdev(struct v4l2_subdev *sd, struct module *module) { struct v4l2_async_notifier *subdev_notifier; struct v4l2_async_notifier *notifier; @@ -807,6 +807,8 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd) return -EINVAL; } + sd->owner = module; + mutex_lock(&list_lock); list_for_each_entry(notifier, ¬ifier_list, notifier_entry) { @@ -849,9 +851,11 @@ err_unbind: mutex_unlock(&list_lock); + sd->owner = NULL; + return ret; } -EXPORT_SYMBOL(v4l2_async_register_subdev); +EXPORT_SYMBOL(__v4l2_async_register_subdev); void v4l2_async_unregister_subdev(struct v4l2_subdev *sd) { diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h index 9bd326d31181..f26c323e9c96 100644 --- a/include/media/v4l2-async.h +++ b/include/media/v4l2-async.h @@ -310,7 +310,9 @@ void v4l2_async_nf_cleanup(struct v4l2_async_notifier *notifier); * * @sd: pointer to &struct v4l2_subdev */ -int v4l2_async_register_subdev(struct v4l2_subdev *sd); +#define v4l2_async_register_subdev(sd) \ + __v4l2_async_register_subdev(sd, THIS_MODULE) +int __v4l2_async_register_subdev(struct v4l2_subdev *sd, struct module *module); /** * v4l2_async_register_subdev_sensor - registers a sensor sub-device to the -- cgit v1.2.3-59-g8ed1b From 2c7f7a3bfe2ab41d070913c2acde269e1ffeb3e2 Mon Sep 17 00:00:00 2001 From: Ricardo Ribalda Date: Mon, 15 Apr 2024 19:34:26 +0000 Subject: media: v4l: async: refactor v4l2_async_create_ancillary_links Return 0 without checking IS_ERR or PTR_ERR if CONFIG_MEDIA_CONTROLLER is not enabled. This makes cocci happier: drivers/media/v4l2-core/v4l2-async.c:331:23-30: ERROR: PTR_ERR applied after initialization to constant on line 319 Signed-off-by: Ricardo Ribalda Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-async.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/media/v4l2-core/v4l2-async.c') diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 3a22da443a22..222f01665f7c 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -316,9 +316,8 @@ v4l2_async_nf_try_all_subdevs(struct v4l2_async_notifier *notifier); static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n, struct v4l2_subdev *sd) { - struct media_link *link = NULL; - #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER) + struct media_link *link; if (sd->entity.function != MEDIA_ENT_F_LENS && sd->entity.function != MEDIA_ENT_F_FLASH) @@ -326,9 +325,10 @@ static int v4l2_async_create_ancillary_links(struct v4l2_async_notifier *n, link = media_create_ancillary_link(&n->sd->entity, &sd->entity); -#endif - return IS_ERR(link) ? PTR_ERR(link) : 0; +#else + return 0; +#endif } static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier, -- cgit v1.2.3-59-g8ed1b