linux-kernel - Re: [PATCH v14 14/56] media: sti: hva: Use vb2_get_buffer() instead of directly access to buffers array

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 9 Nov 2023 10:29:59 +0100
From:   Andrzej Pietrasiewicz <andrzej.p@...labora.com>
To:     Benjamin Gaignard <benjamin.gaignard@...labora.com>,
        mchehab@...nel.org, tfiga@...omium.org, m.szyprowski@...sung.com,
        ming.qian@....com, ezequiel@...guardiasur.com.ar,
        p.zabel@...gutronix.de, gregkh@...uxfoundation.org,
        hverkuil-cisco@...all.nl, nicolas.dufresne@...labora.com
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
        linux-rockchip@...ts.infradead.org, linux-staging@...ts.linux.dev,
        kernel@...labora.com,
        Jean-Christophe Trotin <jean-christophe.trotin@...s.st.com>
Subject: Re: [PATCH v14 14/56] media: sti: hva: Use vb2_get_buffer() instead
 of directly access to buffers array

Hi Benjamin,

W dniu 31.10.2023 o 17:30, Benjamin Gaignard pisze:
> Use vb2_get_buffer() instead of direct access to the vb2_queue bufs array.
> This allows us to change the type of the bufs in the future.
> After each call to vb2_get_buffer() we need to be sure that we get
> a valid pointer so check the return value of all of them.
> Remove index range test since it is done by vb2_get_buffer().

Actually, the patch uses vb2_get_buffer() instead of using vb2_get_buffer().
IOW vb2_get_buffer() continues to be used before and after this patch is applied.

I'd rather reformulate the commit message body to say that we remove
index check because it is already performed by vb2_get_buffer(), but
introduce a check for a NULL result.

Regards,

Andrzej

> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
> CC: Jean-Christophe Trotin <jean-christophe.trotin@...s.st.com>
> ---
>   drivers/media/platform/st/sti/hva/hva-v4l2.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/st/sti/hva/hva-v4l2.c b/drivers/media/platform/st/sti/hva/hva-v4l2.c
> index 3a848ca32a0e..cfe83e9dc01b 100644
> --- a/drivers/media/platform/st/sti/hva/hva-v4l2.c
> +++ b/drivers/media/platform/st/sti/hva/hva-v4l2.c
> @@ -569,14 +569,11 @@ static int hva_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
>   		struct vb2_buffer *vb2_buf;
>   
>   		vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, buf->type);
> -
> -		if (buf->index >= vq->num_buffers) {
> -			dev_dbg(dev, "%s buffer index %d out of range (%d)\n",
> -				ctx->name, buf->index, vq->num_buffers);
> +		vb2_buf = vb2_get_buffer(vq, buf->index);
> +		if (!vb2_buf) {
> +			dev_dbg(dev, "%s buffer index %d not found\n", ctx->name, buf->index);
>   			return -EINVAL;
>   		}
> -
> -		vb2_buf = vb2_get_buffer(vq, buf->index);
>   		stream = to_hva_stream(to_vb2_v4l2_buffer(vb2_buf));
>   		stream->bytesused = buf->bytesused;
>   	}