[PATCH 5/7] media: rzv2h-ivc: Fix concurrent buffer list access

Jacopo Mondi posted 7 patches 3 weeks, 4 days ago
[PATCH 5/7] media: rzv2h-ivc: Fix concurrent buffer list access
Posted by Jacopo Mondi 3 weeks, 4 days ago
From: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>

The list of buffers (`rzv2h_ivc::buffers.queue`) is protected by a
spinlock (`rzv2h_ivc::buffers.lock`). However, in
`rzv2h_ivc_transfer_buffer()`, which runs in a separate workqueue, the
`list_del()` call is executed without holding the spinlock, which makes
it possible for the list to be concurrently modified

Fix that by removing a buffer from the list in the lock protected section.

Cc: stable@vger.kernel.org
Fixes: f0b3984d821b ("media: platform: Add Renesas Input Video Control block driver")
Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
[assign ivc->buffers.curr in critical section as reported by Barnabas]
Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
index 9b75e4b10e99..a22aee0fe1cf 100644
--- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
@@ -153,14 +153,13 @@ static void rzv2h_ivc_transfer_buffer(struct work_struct *work)
 	scoped_guard(spinlock_irqsave, &ivc->buffers.lock) {
 		buf = list_first_entry_or_null(&ivc->buffers.queue,
 					       struct rzv2h_ivc_buf, queue);
-	}
-
-	if (!buf)
-		return;
+		if (!buf)
+			return;
 
-	list_del(&buf->queue);
+		list_del(&buf->queue);
+		ivc->buffers.curr = buf;
+	}
 
-	ivc->buffers.curr = buf;
 	buf->addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
 	rzv2h_ivc_write(ivc, RZV2H_IVC_REG_AXIRX_SADDL_P0, buf->addr);
 

-- 
2.53.0

Re: [PATCH 5/7] media: rzv2h-ivc: Fix concurrent buffer list access
Posted by Dan Scally 3 weeks, 3 days ago
Hi Jacopo and Barnabás

On 13/03/2026 11:14, Jacopo Mondi wrote:
> From: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
> 
> The list of buffers (`rzv2h_ivc::buffers.queue`) is protected by a
> spinlock (`rzv2h_ivc::buffers.lock`). However, in
> `rzv2h_ivc_transfer_buffer()`, which runs in a separate workqueue, the
> `list_del()` call is executed without holding the spinlock, which makes
> it possible for the list to be concurrently modified
> 
> Fix that by removing a buffer from the list in the lock protected section.
> 
> Cc: stable@vger.kernel.org
> Fixes: f0b3984d821b ("media: platform: Add Renesas Input Video Control block driver")
> Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
> [assign ivc->buffers.curr in critical section as reported by Barnabas]
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---

Looks good, thanks

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> index 9b75e4b10e99..a22aee0fe1cf 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> @@ -153,14 +153,13 @@ static void rzv2h_ivc_transfer_buffer(struct work_struct *work)
>   	scoped_guard(spinlock_irqsave, &ivc->buffers.lock) {
>   		buf = list_first_entry_or_null(&ivc->buffers.queue,
>   					       struct rzv2h_ivc_buf, queue);
> -	}
> -
> -	if (!buf)
> -		return;
> +		if (!buf)
> +			return;
>   
> -	list_del(&buf->queue);
> +		list_del(&buf->queue);
> +		ivc->buffers.curr = buf;
> +	}
>   
> -	ivc->buffers.curr = buf;
>   	buf->addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
>   	rzv2h_ivc_write(ivc, RZV2H_IVC_REG_AXIRX_SADDL_P0, buf->addr);
>   
>