Re: [PATCH 14/57] media: atomisp: Check buffer index is in range inside atomisp_qbuf_wrapper()

Hans de Goede
2023-01-24T11:10:30Z

Hi,

On 1/23/23 15:38, Andy Shevchenko wrote:
> On Mon, Jan 23, 2023 at 01:51:22PM +0100, Hans de Goede wrote:
>> Check buffer index is in range inside atomisp_qbuf_wrapper() before
>> using it do index pipe->frame_request_config_id[].
>
> ...
>
>> /* FIXME this abuse of buf->reserved2 comes from the original atomisp buffer handling */
>
> Does the comment belong to this check?

Yes and no, the whole reason we need a wrapper at all is because
of the reserved2 abuse; and likewise the index check is also only
necessary because of the code below using index. If it was not
for that, then we could simply rely on the identical index check in
vb2_ioctl_qbuf() itself.

Before sending this to Mauro I'll amend this to replace this comment
with a comment above the entire wrapper function explaining that
the entire wrapper should eventually be removed.

Regards,

Hans

>
>> + if (buf->index >= vdev->queue->num_buffers)
>> + return -EINVAL;
>> +
>> if (!atomisp_is_vf_pipe(pipe) &&
>> (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) {
>> /* this buffer will have a per-frame parameter */
>