From 5e1a93d61dc439f328a5cecdf60a62d92319770b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= <pobrn@protonmail.com>
Date: Thu, 14 Oct 2021 20:57:23 +0200
Subject: [PATCH] pipewire-v4l2: only access `globals.fd_maps` while holding
 `globals.lock`

`fd_map` structures are allocated in a `pw_array`. When inserting into
a full `pw_array`, it is resized to accomodate the new elements. In that case,
all `fd_map` pointers may be invalidated. Hence it is only safe to access
`fd_map` structs while holding `globals.lock`, which prevents any modifications
to the fd map array, thus keeping the references valid.
---
 pipewire-v4l2/src/pipewire-v4l2.c | 67 ++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/pipewire-v4l2/src/pipewire-v4l2.c b/pipewire-v4l2/src/pipewire-v4l2.c
index a91e71573..c6cbb787e 100644
--- a/pipewire-v4l2/src/pipewire-v4l2.c
+++ b/pipewire-v4l2/src/pipewire-v4l2.c
@@ -299,37 +299,55 @@ static int add_fd_map(int fd, struct file *file)
 	pthread_mutex_unlock(&globals.lock);
 	return 0;
 }
-static struct fd_map *find_fd_map(int fd)
+
+/* must be called with `globals.lock` held */
+static struct fd_map *find_fd_map_unlocked(int fd)
 {
-	struct fd_map *map, *res = NULL;
-	pthread_mutex_lock(&globals.lock);
+	struct fd_map *map;
+
 	pw_array_for_each(map, &globals.fd_maps) {
 		if (map->fd == fd) {
 			ATOMIC_INC(map->file->ref);
-			res =  map;
-			break;
+			return map;
 		}
 	}
-	pthread_mutex_unlock(&globals.lock);
-	return res;
+
+	return NULL;
 }
 
 static struct file *find_file(int fd)
 {
-	struct fd_map *map = find_fd_map(fd);
-	if (map == NULL)
-		return NULL;
-	return map->file;
-}
-
-static void remove_fd_map(struct fd_map *map)
-{
-	struct file *file = map->file;
 	pthread_mutex_lock(&globals.lock);
-	pw_array_remove(&globals.fd_maps, map);
+
+	struct fd_map *map = find_fd_map_unlocked(fd);
+	struct file *file = NULL;
+
+	if (map != NULL)
+		file = map->file;
+
 	pthread_mutex_unlock(&globals.lock);
 
-	unref_file(file);
+	return file;
+}
+
+static struct file *remove_fd_map(int fd)
+{
+	pthread_mutex_lock(&globals.lock);
+
+	struct fd_map *map = find_fd_map_unlocked(fd);
+	struct file *file = NULL;
+
+	if (map != NULL) {
+		file = map->file;
+		pw_array_remove(&globals.fd_maps, map);
+	}
+
+	pthread_mutex_unlock(&globals.lock);
+
+	if (file != NULL)
+		unref_file(file);
+
+	return file;
 }
 
 static int add_file_map(void *addr, struct file *file)
@@ -711,20 +729,15 @@ static int v4l2_dup(int oldfd)
 
 static int v4l2_close(int fd)
 {
-	int res = 0;
-	struct file *file;
-	struct fd_map *map;
+	struct file *file = remove_fd_map(fd);
 
-	if ((map = find_fd_map(fd)) == NULL)
+	if (file == NULL)
 		return globals.old_fops.close(fd);
 
-	file = map->file;
-	remove_fd_map(map);
 	unref_file(file);
 
-	pw_log_info("fd:%d -> %d (%s)", fd,
-			res, strerror(res < 0 ? errno : 0));
-	return res;
+	pw_log_info("fd:%d closed", fd);
+	return 0;
 }
 
 #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))