lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250901-allegro-dvt-fixes-v1-3-4e4d493836ef@emfend.at>
Date: Mon, 01 Sep 2025 17:13:38 +0200
From: Matthias Fend <matthias.fend@...end.at>
To: Michael Tretter <m.tretter@...gutronix.de>, 
 Pengutronix Kernel Team <kernel@...gutronix.de>, 
 Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Matthias Fend <matthias.fend@...end.at>
Subject: [PATCH 3/3] media: allegro: fix race conditions in channel
 handling

Since the channel list is used in different contexts, it must be ensured
that it is always consistent. Also, the channels contained in the list may
only be released when they are no longer needed in any context.

Add a lock to protect the list and reference handling for the channels.

Signed-off-by: Matthias Fend <matthias.fend@...end.at>
---
 drivers/media/platform/allegro-dvt/allegro-core.c | 64 ++++++++++++++++++-----
 1 file changed, 50 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
index d3aea46b7d1d9854307d61d2f1647eaa340d504d..adcaa4f877df1c58807d62ddf3eb21848fb08520 100644
--- a/drivers/media/platform/allegro-dvt/allegro-core.c
+++ b/drivers/media/platform/allegro-dvt/allegro-core.c
@@ -177,6 +177,7 @@ struct allegro_dev {
 	 */
 	unsigned long channel_user_ids;
 	struct list_head channels;
+	struct mutex channels_lock;
 };
 
 static const struct regmap_config allegro_regmap_config = {
@@ -200,6 +201,7 @@ static const struct regmap_config allegro_sram_config = {
 #define fh_to_channel(__fh) container_of(__fh, struct allegro_channel, fh)
 
 struct allegro_channel {
+	struct kref ref;
 	struct allegro_dev *dev;
 	struct v4l2_fh fh;
 	struct v4l2_ctrl_handler ctrl_handler;
@@ -427,33 +429,55 @@ static unsigned long allegro_next_user_id(struct allegro_dev *dev)
 }
 
 static struct allegro_channel *
-allegro_find_channel_by_user_id(struct allegro_dev *dev,
-				unsigned int user_id)
+allegro_ref_get_channel_by_user_id(struct allegro_dev *dev,
+				   unsigned int user_id)
 {
 	struct allegro_channel *channel;
 
+	guard(mutex)(&dev->channels_lock);
+
 	list_for_each_entry(channel, &dev->channels, list) {
-		if (channel->user_id == user_id)
-			return channel;
+		if (channel->user_id == user_id) {
+			if (kref_get_unless_zero(&channel->ref))
+				return channel;
+			break;
+		}
 	}
 
 	return ERR_PTR(-EINVAL);
 }
 
 static struct allegro_channel *
-allegro_find_channel_by_channel_id(struct allegro_dev *dev,
-				   unsigned int channel_id)
+allegro_ref_get_channel_by_channel_id(struct allegro_dev *dev,
+				      unsigned int channel_id)
 {
 	struct allegro_channel *channel;
 
+	guard(mutex)(&dev->channels_lock);
+
 	list_for_each_entry(channel, &dev->channels, list) {
-		if (channel->mcu_channel_id == channel_id)
-			return channel;
+		if (channel->mcu_channel_id == channel_id) {
+			if (kref_get_unless_zero(&channel->ref))
+				return channel;
+			break;
+		}
 	}
 
 	return ERR_PTR(-EINVAL);
 }
 
+static void allegro_free_channel(struct kref *ref)
+{
+	struct allegro_channel *channel = container_of(ref, struct allegro_channel, ref);
+
+	kfree(channel);
+}
+
+static int allegro_ref_put_channel(struct allegro_channel *channel)
+{
+	return kref_put(&channel->ref, allegro_free_channel);
+}
+
 static inline bool channel_exists(struct allegro_channel *channel)
 {
 	return channel->mcu_channel_id != -1;
@@ -2183,7 +2207,7 @@ allegro_handle_create_channel(struct allegro_dev *dev,
 	int err = 0;
 	struct create_channel_param param;
 
-	channel = allegro_find_channel_by_user_id(dev, msg->user_id);
+	channel = allegro_ref_get_channel_by_user_id(dev, msg->user_id);
 	if (IS_ERR(channel)) {
 		v4l2_warn(&dev->v4l2_dev,
 			  "received %s for unknown user %d\n",
@@ -2250,6 +2274,7 @@ allegro_handle_create_channel(struct allegro_dev *dev,
 out:
 	channel->error = err;
 	complete(&channel->completion);
+	allegro_ref_put_channel(channel);
 
 	/* Handled successfully, error is passed via channel->error */
 	return 0;
@@ -2261,7 +2286,7 @@ allegro_handle_destroy_channel(struct allegro_dev *dev,
 {
 	struct allegro_channel *channel;
 
-	channel = allegro_find_channel_by_channel_id(dev, msg->channel_id);
+	channel = allegro_ref_get_channel_by_channel_id(dev, msg->channel_id);
 	if (IS_ERR(channel)) {
 		v4l2_err(&dev->v4l2_dev,
 			 "received %s for unknown channel %d\n",
@@ -2274,6 +2299,7 @@ allegro_handle_destroy_channel(struct allegro_dev *dev,
 		 "user %d: vcu destroyed channel %d\n",
 		 channel->user_id, channel->mcu_channel_id);
 	complete(&channel->completion);
+	allegro_ref_put_channel(channel);
 
 	return 0;
 }
@@ -2284,7 +2310,7 @@ allegro_handle_encode_frame(struct allegro_dev *dev,
 {
 	struct allegro_channel *channel;
 
-	channel = allegro_find_channel_by_channel_id(dev, msg->channel_id);
+	channel = allegro_ref_get_channel_by_channel_id(dev, msg->channel_id);
 	if (IS_ERR(channel)) {
 		v4l2_err(&dev->v4l2_dev,
 			 "received %s for unknown channel %d\n",
@@ -2294,6 +2320,7 @@ allegro_handle_encode_frame(struct allegro_dev *dev,
 	}
 
 	allegro_channel_finish_frame(channel, msg);
+	allegro_ref_put_channel(channel);
 
 	return 0;
 }
@@ -3079,6 +3106,8 @@ static int allegro_open(struct file *file)
 	if (!channel)
 		return -ENOMEM;
 
+	kref_init(&channel->ref);
+
 	v4l2_fh_init(&channel->fh, vdev);
 
 	init_completion(&channel->completion);
@@ -3245,7 +3274,10 @@ static int allegro_open(struct file *file)
 		goto error;
 	}
 
-	list_add(&channel->list, &dev->channels);
+	scoped_guard(mutex, &dev->channels_lock) {
+		list_add(&channel->list, &dev->channels);
+	}
+
 	file->private_data = &channel->fh;
 	v4l2_fh_add(&channel->fh);
 
@@ -3262,17 +3294,20 @@ static int allegro_open(struct file *file)
 static int allegro_release(struct file *file)
 {
 	struct allegro_channel *channel = fh_to_channel(file->private_data);
+	struct allegro_dev *dev = channel->dev;
 
 	v4l2_m2m_ctx_release(channel->fh.m2m_ctx);
 
-	list_del(&channel->list);
+	scoped_guard(mutex, &dev->channels_lock) {
+		list_del(&channel->list);
+	}
 
 	v4l2_ctrl_handler_free(&channel->ctrl_handler);
 
 	v4l2_fh_del(&channel->fh);
 	v4l2_fh_exit(&channel->fh);
 
-	kfree(channel);
+	allegro_ref_put_channel(channel);
 
 	return 0;
 }
@@ -3867,6 +3902,7 @@ static int allegro_probe(struct platform_device *pdev)
 	dev->plat_dev = pdev;
 	init_completion(&dev->init_complete);
 	INIT_LIST_HEAD(&dev->channels);
+	mutex_init(&dev->channels_lock);
 
 	mutex_init(&dev->lock);
 

-- 
2.25.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ