diff mbox series

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

Message ID 20230123125205.622152-15-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series media: atomisp: Big power-management changes + lots of fixes | expand

Commit Message

Hans de Goede Jan. 23, 2023, 12:51 p.m. UTC
Check buffer index is in range inside atomisp_qbuf_wrapper() before
using it do index pipe->frame_request_config_id[].

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Andy Shevchenko Jan. 23, 2023, 2:38 p.m. UTC | #1
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?

> +	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 */
Hans de Goede Jan. 24, 2023, 11:09 a.m. UTC | #2
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 */
>
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 9cb9685d91bb..d0dd3dbd6f6a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1056,6 +1056,9 @@  static int atomisp_qbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer
 	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
 
 	/* FIXME this abuse of buf->reserved2 comes from the original atomisp buffer handling */
+	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 */