From 47b037a0512d9f8675ec2693bed46c8ea6a884ab Mon Sep 17 00:00:00 2001 From: Tuukka Toivonen Date: Fri, 27 Jan 2017 08:32:56 -0200 Subject: [PATCH] [media] v4l2-async: failing functions shouldn't have side effects v4l2-async had several functions doing some operations and then not undoing the operations in a failure situation. For example, v4l2_async_test_notify() moved a subdev into notifier's done list even if registering the subdev (v4l2_device_register_subdev) failed. If the subdev was allocated and v4l2_async_register_subdev() called from the driver's probe() function, as usually, the probe() function freed the allocated subdev and returned a failure. Nevertheless, the subdev was still left into the notifier's done list, causing an access to already freed memory when the notifier was later unregistered. A hand-edited call trace leaving freed subdevs into the notifier: v4l2_async_register_notifier(notifier, asd) cameradrv_probe sd = devm_kzalloc() v4l2_async_register_subdev(sd) v4l2_async_test_notify(notifier, sd, asd) list_move(sd, ¬ifier->done) v4l2_device_register_subdev(notifier->v4l2_dev, sd) cameradrv_registered(sd) -> fails ->v4l2_async_register_subdev returns failure ->cameradrv_probe returns failure ->devres frees the allocated sd ->sd was freed but it still remains in the notifier's list. This patch fixes this and several other cases where a failing function could leave nodes into a linked list while the caller might free the node due to a failure. Signed-off-by: Tuukka Toivonen Acked-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-async.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 618135089548..96cc733f35ef 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -100,18 +100,11 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier, { int ret; - /* Remove from the waiting list */ - list_del(&asd->list); - sd->asd = asd; - sd->notifier = notifier; - if (notifier->bound) { ret = notifier->bound(notifier, sd, asd); if (ret < 0) return ret; } - /* Move from the global subdevice list to notifier's done */ - list_move(&sd->async_list, ¬ifier->done); ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd); if (ret < 0) { @@ -120,6 +113,14 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier, return ret; } + /* Remove from the waiting list */ + list_del(&asd->list); + sd->asd = asd; + sd->notifier = notifier; + + /* Move from the global subdevice list to notifier's done */ + list_move(&sd->async_list, ¬ifier->done); + if (list_empty(¬ifier->waiting) && notifier->complete) return notifier->complete(notifier); @@ -169,9 +170,6 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, mutex_lock(&list_lock); - /* Keep also completed notifiers on the list */ - list_add(¬ifier->list, ¬ifier_list); - list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { int ret; @@ -186,6 +184,9 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, } } + /* Keep also completed notifiers on the list */ + list_add(¬ifier->list, ¬ifier_list); + mutex_unlock(&list_lock); return 0; -- 2.20.1