Commit 47b037a0 authored by Tuukka Toivonen's avatar Tuukka Toivonen Committed by Mauro Carvalho Chehab

[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, &notifier->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: default avatarTuukka Toivonen <tuukka.toivonen@intel.com>
Acked-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@s-opensource.com>
parent 79a2eda8
...@@ -100,18 +100,11 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier, ...@@ -100,18 +100,11 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
{ {
int ret; int ret;
/* Remove from the waiting list */
list_del(&asd->list);
sd->asd = asd;
sd->notifier = notifier;
if (notifier->bound) { if (notifier->bound) {
ret = notifier->bound(notifier, sd, asd); ret = notifier->bound(notifier, sd, asd);
if (ret < 0) if (ret < 0)
return ret; return ret;
} }
/* Move from the global subdevice list to notifier's done */
list_move(&sd->async_list, &notifier->done);
ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd); ret = v4l2_device_register_subdev(notifier->v4l2_dev, sd);
if (ret < 0) { if (ret < 0) {
...@@ -120,6 +113,14 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier, ...@@ -120,6 +113,14 @@ static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
return ret; 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, &notifier->done);
if (list_empty(&notifier->waiting) && notifier->complete) if (list_empty(&notifier->waiting) && notifier->complete)
return notifier->complete(notifier); return notifier->complete(notifier);
...@@ -169,9 +170,6 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, ...@@ -169,9 +170,6 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
mutex_lock(&list_lock); mutex_lock(&list_lock);
/* Keep also completed notifiers on the list */
list_add(&notifier->list, &notifier_list);
list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) { list_for_each_entry_safe(sd, tmp, &subdev_list, async_list) {
int ret; int ret;
...@@ -186,6 +184,9 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, ...@@ -186,6 +184,9 @@ int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
} }
} }
/* Keep also completed notifiers on the list */
list_add(&notifier->list, &notifier_list);
mutex_unlock(&list_lock); mutex_unlock(&list_lock);
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment