[PATCH AUTOSEL 6.19-6.12] media: ipu6: Ensure stream_mutex is acquired when dealing with node list

[Sasha Levin <sashal@kernel.org>]

From: Sakari Ailus <sakari.ailus@linux.intel.com>

[ Upstream commit 779bdaad2abf718fb8116839e818e58852874b4d ]

The ipu6 isys driver maintains the list of video buffer queues related to
a stream (in ipu6 context streams on the same CSI-2 virtual channel) and
this list is modified through VIDIOC_STREAMON and VIDIOC_STREAMOFF IOCTLs.
Ensure the common mutex is acquired when accessing the linked list, i.e.
the isys device context's stream_mutex.

Add a lockdep assert to ipu6_isys_get_buffer_list() and switch to guard()
while at it as the error handling becomes more simple this way.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of commit: "media: ipu6: Ensure stream_mutex is acquired
when dealing with node list"

### 1. COMMIT MESSAGE ANALYSIS

The commit message clearly describes a **synchronization fix**: the ipu6
ISYS driver maintains a linked list of video buffer queues related to a
stream, and this list is modified through `STREAMON`/`STREAMOFF` ioctls.
The list was being accessed without proper mutex protection, creating a
race condition. The fix ensures `stream_mutex` is held when accessing
the linked list.

Key phrases: "Ensure the common mutex is acquired when accessing the
linked list" — this is a classic race condition fix.

### 2. CODE CHANGE ANALYSIS

The commit makes three distinct changes:

**Change 1: lockdep_assert_held() in buffer_list_get()**
```c
+       lockdep_assert_held(&stream->mutex);
```
This adds a runtime assertion that `stream->mutex` is held when
`buffer_list_get()` is called. This is a debugging aid that documents
the locking requirement. The function iterates over `stream->queues` (a
linked list) via `list_for_each_entry(aq, &stream->queues, node)`, which
needs protection.

**Change 2: guard(mutex) in ipu6_isys_stream_start()**
```c
- mutex_lock(&stream->isys->stream_mutex);
+       guard(mutex)(&stream->isys->stream_mutex);
        ret = ipu6_isys_video_set_streaming(av, 1, bl);
- mutex_unlock(&stream->isys->stream_mutex);
```
This converts a manual lock/unlock pair to a `guard(mutex)` (scoped
lock), which extends the lock's lifetime through the entire function
scope. This is a **critical fix**: previously, `stream_mutex` was
released immediately after `ipu6_isys_video_set_streaming()`, but then
`buffer_list_get()` was called in the `do...while` loop below without
holding `stream_mutex`. Since `buffer_list_get()` iterates over
`stream->queues` (a linked list that can be modified by
`stop_streaming`), this was a real race condition.

The `buffer_list_get()` now has `lockdep_assert_held(&stream->mutex)` —
but wait, it asserts `stream->mutex`, not `stream->isys->stream_mutex`.
Let me look more carefully...

Actually, looking at the `buffer_list_get` function, it asserts
`stream->mutex`, while the `ipu6_isys_stream_start` acquires
`stream->isys->stream_mutex`. These are different mutexes. The
`stream->mutex` would need to be held by the caller of
`ipu6_isys_stream_start()`. The `stream_mutex` change in
`ipu6_isys_stream_start` is about protecting the `list_del` in
`stop_streaming`.

**Change 3: Moving list_del() inside stream_mutex in stop_streaming()**
```c
        mutex_lock(&av->isys->stream_mutex);
        if (stream->nr_streaming == stream->nr_queues &&
stream->streaming)
                ipu6_isys_video_set_streaming(av, 0, NULL);
+       list_del(&aq->node);
        mutex_unlock(&av->isys->stream_mutex);

        stream->nr_streaming--;
- list_del(&aq->node);
```
This is **the most important change**. Previously, `list_del(&aq->node)`
was called **outside** the `stream_mutex` protection. This means if one
thread is in `stop_streaming` doing `list_del()` while another thread is
in `ipu6_isys_stream_start` iterating over the list (in
`buffer_list_get`), there's a **use-after-free or list corruption race
condition**.

By moving `list_del()` inside the `stream_mutex` lock, and extending
`stream_mutex` in `ipu6_isys_stream_start` to cover the
`buffer_list_get` calls, the list is now properly protected.

### 3. BUG CLASSIFICATION

This is a **race condition fix** on a linked list that is concurrently
modified (via `list_del`) and read (via `list_for_each_entry`). This
class of bug can cause:
- **List corruption** (corrupted forward/backward pointers)
- **Use-after-free** (iterating over a node that was just removed and
  potentially freed)
- **Kernel crash/oops** (dereferencing corrupted pointers)

The race window is between `STREAMON` and `STREAMOFF` ioctls, which can
be triggered from userspace on different file descriptors of the same
stream.

### 4. SCOPE AND RISK ASSESSMENT

- **Files changed**: 1 file (`ipu6-isys-queue.c`)
- **Lines changed**: Very small — adding a lockdep assert, converting
  lock/unlock to guard, moving one `list_del` inside an existing lock
  scope
- **Risk**: LOW — the changes are straightforward synchronization fixes:
  - The lockdep assert is purely a debugging check
  - The guard(mutex) extends lock scope (safe — just holds lock longer)
  - Moving list_del inside existing lock scope is a well-understood
    pattern
- **Subsystem**: Intel IPU6 camera driver (media/pci/intel/ipu6) —
  relatively contained

### 5. DEPENDENCY CHECK

The `guard(mutex)` macro requires `linux/cleanup.h` which was added in
Linux 6.5. The IPU6 driver was added relatively recently. Need to check
if the affected code exists in stable trees.

The IPU6 driver (`drivers/media/pci/intel/ipu6/`) was merged in the 6.10
timeframe. So this would only apply to 6.12.y and possibly 6.11.y stable
trees (if they exist).

The `guard()` macro dependency (`linux/cleanup.h`) is available in these
kernels, so no backport adjustment needed.

### 6. STABILITY AND TRUST INDICATORS

- **Author**: Sakari Ailus — well-known Linux media subsystem maintainer
  at Intel
- **Reviewer**: Hans Verkuil — another prominent media maintainer
- **The fix is obviously correct**: Moving `list_del` inside an existing
  lock that protects the same list is a textbook race condition fix

### 7. USER IMPACT

Intel IPU6 is the camera subsystem used in modern Intel laptops (Alder
Lake, Raptor Lake, etc.). Users doing camera streaming could hit this
race condition when starting/stopping video capture, leading to kernel
crashes. This affects laptops using the IPU6 camera with the mainline
driver.

### CONCLUSION

This commit fixes a real race condition (concurrent list modification
and traversal) that can lead to list corruption, use-after-free, or
kernel crashes. The fix is:
- **Small and surgical**: One lockdep assert, one lock scope change, one
  line moved inside existing lock
- **Obviously correct**: Classic pattern of protecting shared data
  structures with proper locking
- **Low risk**: Only extends lock scope and moves an operation inside an
  existing lock
- **Authored by subsystem maintainer**: High confidence in correctness
- **Fixes a real bug**: Race between STREAMON/STREAMOFF ioctls on shared
  stream list

The fix meets all stable kernel criteria.

**YES**

 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index aa2cf7287477c..8f05987cdb4e7 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2013--2024 Intel Corporation
  */
 #include <linux/atomic.h>
+#include <linux/cleanup.h>
 #include <linux/bug.h>
 #include <linux/device.h>
 #include <linux/list.h>
@@ -201,6 +202,8 @@ static int buffer_list_get(struct ipu6_isys_stream *stream,
 	unsigned long flags;
 	unsigned long buf_flag = IPU6_ISYS_BUFFER_LIST_FL_INCOMING;
 
+	lockdep_assert_held(&stream->mutex);
+
 	bl->nbufs = 0;
 	INIT_LIST_HEAD(&bl->head);
 
@@ -294,9 +297,8 @@ static int ipu6_isys_stream_start(struct ipu6_isys_video *av,
 	struct ipu6_isys_buffer_list __bl;
 	int ret;
 
-	mutex_lock(&stream->isys->stream_mutex);
+	guard(mutex)(&stream->isys->stream_mutex);
 	ret = ipu6_isys_video_set_streaming(av, 1, bl);
-	mutex_unlock(&stream->isys->stream_mutex);
 	if (ret)
 		goto out_requeue;
 
@@ -637,10 +639,10 @@ static void stop_streaming(struct vb2_queue *q)
 	mutex_lock(&av->isys->stream_mutex);
 	if (stream->nr_streaming == stream->nr_queues && stream->streaming)
 		ipu6_isys_video_set_streaming(av, 0, NULL);
+	list_del(&aq->node);
 	mutex_unlock(&av->isys->stream_mutex);
 
 	stream->nr_streaming--;
-	list_del(&aq->node);
 	stream->streaming = 0;
 	mutex_unlock(&stream->mutex);
 
-- 
2.51.0