staging: media: tegra-video: fix infinite recursion regression

Commit Message

Luca Ceresoli Sept. 26, 2023, 10:03 a.m. UTC

Since commit 9bf19fbf0c8b ("media: v4l: async: Rework internal lists"), aka
v6.6-rc1~97^2~198, probing the tegra-video VI driver causes infinite
recursion due tegra_vi_graph_parse_one() calling itself until:

[    1.571168] Insufficient stack space to handle exception!
...
[    1.591416] Internal error: kernel stack overflow: 0 [#1] PREEMPT SMP ARM
...
[    3.861013]  of_phandle_iterator_init from __of_parse_phandle_with_args+0x40/0xf0
[    3.868497]  __of_parse_phandle_with_args from of_fwnode_graph_get_remote_endpoint+0x68/0xa8
[    3.876938]  of_fwnode_graph_get_remote_endpoint from fwnode_graph_get_remote_port_parent+0x30/0x7c
[    3.885984]  fwnode_graph_get_remote_port_parent from tegra_vi_graph_parse_one+0x7c/0x224
[    3.894158]  tegra_vi_graph_parse_one from tegra_vi_graph_parse_one+0x144/0x224
[    3.901459]  tegra_vi_graph_parse_one from tegra_vi_graph_parse_one+0x144/0x224
[    3.908760]  tegra_vi_graph_parse_one from tegra_vi_graph_parse_one+0x144/0x224
[    3.916061]  tegra_vi_graph_parse_one from tegra_vi_graph_parse_one+0x144/0x224
...
[    4.857892]  tegra_vi_graph_parse_one from tegra_vi_graph_parse_one+0x144/0x224
[    4.865193]  tegra_vi_graph_parse_one from tegra_vi_graph_parse_one+0x144/0x224
[    4.872494]  tegra_vi_graph_parse_one from tegra_vi_init+0x574/0x6d4
[    4.878842]  tegra_vi_init from host1x_device_init+0x84/0x15c
[    4.884594]  host1x_device_init from host1x_video_probe+0xa0/0x114
[    4.890770]  host1x_video_probe from really_probe+0xe0/0x400

The reason is the mentioned commit changed tegra_vi_graph_find_entity() to
search for an entity in the done notifier list:

> @@ -1464,7 +1464,7 @@ tegra_vi_graph_find_entity(struct tegra_vi_channel *chan,
>  	struct tegra_vi_graph_entity *entity;
>  	struct v4l2_async_connection *asd;
>
> -	list_for_each_entry(asd, &chan->notifier.asc_list, asc_entry) {
> +	list_for_each_entry(asd, &chan->notifier.done_list, asc_entry) {
>  		entity = to_tegra_vi_graph_entity(asd);
>  		if (entity->asd.match.fwnode == fwnode)
>  			return entity;

This is not always correct, being tegra_vi_graph_find_entity() called in
three locations, in this order:

 1. tegra_vi_graph_parse_one()    -- called while probing
 2. tegra_vi_graph_notify_bound() -- the .bound notifier op
 3. tegra_vi_graph_build()        -- called in the .complete notifier op

Locations 1 and 2 are called before moving the entity from waiting_list to
done_list, thus they won't find what they are looking for in
done_list. Location 3 happens afterwards and thus it is not broken, however
it means tegra_vi_graph_find_entity() should not search in the same list
every time.

The error appears at step 1: tegra_vi_graph_parse_one() iterates
recursively until it finds the entity already notified, which now never
happens.

Fix by passing the specific notifier list pointer to
tegra_vi_graph_find_entity() instead of the channel, so each caller can
search in whatever list is correct.

Also improve the tegra_vi_graph_find_entity() comment.

Fixes: 9bf19fbf0c8b ("media: v4l: async: Rework internal lists")
Cc: Thierry Reding <[e-mail]>
Cc: Jonathan Hunter <[e-mail]>
Cc: Sowjanya Komatineni <[e-mail]>
Signed-off-by: Luca Ceresoli <[e-mail]>
---
 drivers/staging/media/tegra-video/vi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Patch

diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c
index e98b3010520e..d9a6d5b2aa84 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -1455,17 +1455,18 @@  static int __maybe_unused vi_runtime_suspend(struct device *dev)
 }
 
 /*
- * Graph Management
+ * Find the entity matching a given fwnode in an v4l2_async_notifier list
  */
 static struct tegra_vi_graph_entity *
-tegra_vi_graph_find_entity(struct tegra_vi_channel *chan,
+tegra_vi_graph_find_entity(struct list_head *list,
 			   const struct fwnode_handle *fwnode)
 {
 	struct tegra_vi_graph_entity *entity;
 	struct v4l2_async_connection *asd;
 
-	list_for_each_entry(asd, &chan->notifier.done_list, asc_entry) {
+	list_for_each_entry(asd, list, asc_entry) {
 		entity = to_tegra_vi_graph_entity(asd);
+
 		if (entity->asd.match.fwnode == fwnode)
 			return entity;
 	}
@@ -1532,7 +1533,7 @@  static int tegra_vi_graph_build(struct tegra_vi_channel *chan,
 		}
 
 		/* find the remote entity from notifier list */
-		ent = tegra_vi_graph_find_entity(chan, link.remote_node);
+		ent = tegra_vi_graph_find_entity(&chan->notifier.done_list, link.remote_node);
 		if (!ent) {
 			dev_err(vi->dev, "no entity found for %pOF\n",
 				to_of_node(link.remote_node));
@@ -1664,7 +1665,7 @@  static int tegra_vi_graph_notify_bound(struct v4l2_async_notifier *notifier,
 	 * Locate the entity corresponding to the bound subdev and store the
 	 * subdev pointer.
 	 */
-	entity = tegra_vi_graph_find_entity(chan, subdev->fwnode);
+	entity = tegra_vi_graph_find_entity(&chan->notifier.waiting_list, subdev->fwnode);
 	if (!entity) {
 		dev_err(vi->dev, "no entity for subdev %s\n", subdev->name);
 		return -EINVAL;
@@ -1713,7 +1714,7 @@  static int tegra_vi_graph_parse_one(struct tegra_vi_channel *chan,
 
 		/* skip entities that are already processed */
 		if (device_match_fwnode(vi->dev, remote) ||
-		    tegra_vi_graph_find_entity(chan, remote)) {
+		    tegra_vi_graph_find_entity(&chan->notifier.waiting_list, remote)) {
 			fwnode_handle_put(remote);
 			continue;
 		}