[<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