Re: [PATCH v5 06/29] media: rockchip: rga: fix too small buffer size

[Sven Püschel]

Hi Nicolas,

On 5/8/26 11:11 PM, Nicolas Dufresne wrote:
> Le mardi 28 avril 2026 à 11:00 +0200, Sven Püschel a écrit :
> > Fix the command buffer size being only a quarter of the actual size.
> > The RGA_CMDBUF_SIZE macro was potentially intended to specify the length
> > of the cmdbuf u32 array pointer. But as it's used to specify the size of
> > the allocation, which is counted in bytes. Therefore adjust the macro
> > size to bytes as it better matches the variable name and adjust it's
> > users accordingly.
> >
> > As the command buffer is relatively small, it probably didn't caused
> > an issue due to being smaller than a single page.
> >
> > Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
> > Signed-off-by: Sven Püschel <s.pueschel@xxxxxxxxxxxxxx>
> Reviewed-by: Nicolas Dufresne  <nicolas.dufresne@xxxxxxxxxxxxx>
>
> > ---
> >   drivers/media/platform/rockchip/rga/rga-hw.c | 2 +-
> >   drivers/media/platform/rockchip/rga/rga-hw.h | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
> > index 43ed742a16492..d1618bb247501 100644
> > --- a/drivers/media/platform/rockchip/rga/rga-hw.c
> > +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
> > @@ -414,7 +414,7 @@ static void rga_cmd_set(struct rga_ctx *ctx,
> >   {
> >   	struct rockchip_rga *rga = ctx->rga;
> >   
> > -	memset(rga->cmdbuf_virt, 0, RGA_CMDBUF_SIZE * 4);
> > +	memset(rga->cmdbuf_virt, 0, RGA_CMDBUF_SIZE);
> So we had a buffer overrun ?

From my point of view, yes [1]:

    /* Create CMD buffer */
    rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
                       &rga->cmdbuf_phy, GFP_KERNEL,
                       DMA_ATTR_WRITE_COMBINE);


Given that 0x20 * 4 is smaller than a page, it probably didn't caused an 
issue as we didn't write out of the one page allocated.

Btw.: I've noticed it while reading a bit over the comments from 
sashiko.dev [2]. I'll also add this link for clarity to the comment in 
my next series (seems to be the common way of referring to sashiko.dev).

Sincerely
    Sven


[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/rockchip/rga/rga.c?id=50897c955902c93ae71c38698abb910525ebdc89#n880

[2] 
https://sashiko.dev/#/patchset/20260325-spu-rga3-v4-0-e90ec1c61354%40pengutronix.de?part=10

> Nicolas
>
> >   
> >   	rga_cmd_set_src_addr(ctx, src->dma_desc_pa);
> >   	/*
> > diff --git a/drivers/media/platform/rockchip/rga/rga-hw.h b/drivers/media/platform/rockchip/rga/rga-hw.h
> > index cc6bd7f5b0300..2b8537a5fd0d7 100644
> > --- a/drivers/media/platform/rockchip/rga/rga-hw.h
> > +++ b/drivers/media/platform/rockchip/rga/rga-hw.h
> > @@ -6,7 +6,7 @@
> >   #ifndef __RGA_HW_H__
> >   #define __RGA_HW_H__
> >   
> > -#define RGA_CMDBUF_SIZE 0x20
> > +#define RGA_CMDBUF_SIZE 0x80
> >   
> >   /* Hardware limits */
> >   #define MAX_WIDTH 8192