[PATCH 1/2] media: amd: isp4: hold buf_list_lock while sending buffers at stream start

[Seifelden Abdelmonem]

isp4vid_qops_start_streaming() walks isp_vdev->buf_list without
buf_list_lock and passes each buffer to isp4sd_ioc_send_img_buf().
Every other access to buf_list holds that lock.

vb2 serialises the walk against buf_queue(), but not against the
firmware response thread. Once ISP4SD_MIN_BUF_CNT_BEF_START_STREAM
buffers have been sent, isp4sd_ioc_send_img_buf() also sends
START_STREAM, so the firmware can complete buffers while the walk is
still running. isp4vid_handle_frame_done() then list_del()s the
completed buffer from buf_list. If that buffer is the current entry of
the walk, list_for_each_entry() follows the poisoned next pointer:

  Oops: general protection fault, maybe for address 0xffff8e6944d97060
  RIP: 0010:isp4if_alloc_buffer_node+0x37/0x50 [amd_isp4_capture]
  RBX: dead0000000000b8 ... RSI: dead0000000000b8
  Call Trace:
   isp4sd_ioc_send_img_buf
   isp4vid_qops_start_streaming
   vb2_start_streaming
   vb2_core_streamon

The oopsing task holds the queue lock, so the video device can no
longer be closed and the module cannot be unloaded until reboot.

This was hit on a board that is not supported upstream yet, with an
out-of-tree change that made one isp4sd_ioc_send_img_buf() call take
over a second. With the stock driver the window is much shorter, but
the list is still modified concurrently without the lock.

Hold buf_list_lock across the walk. This adds the lock order
buf_list_lock -> ops_mutex. No path takes them in the reverse order:
isp4vid_handle_frame_done() runs in the response thread without
ops_mutex. isp4sd_ioc_send_img_buf() only queues commands and does not
wait for a firmware response, so the response thread is at most
delayed until the initial buffers have been sent.

Fixes: 2ccf48af2270 ("media: platform: amd: isp4 video node and buffers handling added")
Signed-off-by: Seifelden Abdelmonem <saif.elden.hatim@xxxxxxxxx>
---
Testing and AI use: see the cover letter.

 drivers/media/platform/amd/isp4/isp4_video.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/amd/isp4/isp4_video.c b/drivers/media/platform/amd/isp4/isp4_video.c
index 856a2a0b4..58ea8d6ac 100644
--- a/drivers/media/platform/amd/isp4/isp4_video.c
+++ b/drivers/media/platform/amd/isp4/isp4_video.c
@@ -567,9 +567,11 @@ static int isp4vid_qops_start_streaming(struct vb2_queue *vq,
 		}
 	}
 
-	list_for_each_entry(isp4vid_buf, &isp_vdev->buf_list, list)
-		isp4sd_ioc_send_img_buf(isp_vdev->isp_sdev,
-					&isp4vid_buf->img_buf);
+	scoped_guard(mutex, &isp_vdev->buf_list_lock) {
+		list_for_each_entry(isp4vid_buf, &isp_vdev->buf_list, list)
+			isp4sd_ioc_send_img_buf(isp_vdev->isp_sdev,
+						&isp4vid_buf->img_buf);
+	}
 
 	isp_vdev->stream_started = true;
 
-- 
2.56.0