RE: [PATCH] omap_vout: Added check in reqbuf & mmap for buf_size allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Taneja, Archit
> Sent: Friday, June 17, 2011 3:57 PM
> To: Hiremath, Vaibhav
> Cc: linux-media@xxxxxxxxxxxxxxx; mchehab@xxxxxxxxxx; hverkuil@xxxxxxxxx
> Subject: Re: [PATCH] omap_vout: Added check in reqbuf & mmap for buf_size
> allocation
> 
> Hi,
> 
> On Friday 17 June 2011 03:33 PM, Hiremath, Vaibhav wrote:
> >
> >> -----Original Message-----
> >> From: Taneja, Archit
> >> Sent: Friday, June 17, 2011 2:16 PM
> >> To: Hiremath, Vaibhav
> >> Cc: linux-media@xxxxxxxxxxxxxxx; mchehab@xxxxxxxxxx; hverkuil@xxxxxxxxx
> >> Subject: Re: [PATCH] omap_vout: Added check in reqbuf&  mmap for
> buf_size
> >> allocation
> >>
> >> Hi,
> >>
> >> On Friday 17 June 2011 01:44 AM, Hiremath, Vaibhav wrote:
> >>> From: Vaibhav Hiremath<hvaibhav@xxxxxx>
> >>>
> >>> The usecase where, user allocates small size of buffer
> >>> through bootargs (video1_bufsize/video2_bufsize) and later from
> >> application
> >>> tries to set the format which requires larger buffer size, driver
> >> doesn't
> >>> check for insufficient buffer size and allows application to map extra
> >> buffer.
> >>> This leads to kernel crash, when user application tries to access
> memory
> >>> beyond the allocation size.
> >>
> >> Query: Why do we pass the bufsize as bootargs in the first place? Is it
> >> needed at probe time?
> >>
> > [Hiremath, Vaibhav] Yes, look out for variable
> (video1_bufsize/video2_bufsize) in code.
> 
> Yes, but why do we need to allocate some fixed size buffers at boot
> time? Is it done because it makes our allocation happens faster during
> reqbufs? Or is it required for VRFB?
> 
> Could you explain the reason/startegy behind allocating buffers of a
> particular size at boot time?
> 
[Hiremath, Vaibhav] This is required to get rid of Linux memory fragmentation, user can reserve the memory based on usecase during boot time itself.

Thanks,
Vaibhav

> Thanks,
> Archit
> 
> >
> > Thanks,
> > Vaibhav
> >
> >> Thanks,
> >> Archit
> >>
> >>>
> >>> Added check in both mmap and reqbuf call back function,
> >>> and return error if the size of the buffer allocated by user through
> >>> bootargs is less than the S_FMT size.
> >>>
> >>> Signed-off-by: Vaibhav Hiremath<hvaibhav@xxxxxx>
> >>> ---
> >>>    drivers/media/video/omap/omap_vout.c |   16 ++++++++++++++++
> >>>    1 files changed, 16 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/drivers/media/video/omap/omap_vout.c
> >> b/drivers/media/video/omap/omap_vout.c
> >>> index 3bc909a..343b50c 100644
> >>> --- a/drivers/media/video/omap/omap_vout.c
> >>> +++ b/drivers/media/video/omap/omap_vout.c
> >>> @@ -678,6 +678,14 @@ static int omap_vout_buffer_setup(struct
> >> videobuf_queue *q, unsigned int *count,
> >>>    	startindex = (vout->vid == OMAP_VIDEO1) ?
> >>>    		video1_numbuffers : video2_numbuffers;
> >>>
> >>> +	/* Check the size of the buffer */
> >>> +	if (*size>   vout->buffer_size) {
> >>> +		v4l2_err(&vout->vid_dev->v4l2_dev,
> >>> +				"buffer allocation mismatch [%u] [%u]\n",
> >>> +				*size, vout->buffer_size);
> >>> +		return -ENOMEM;
> >>> +	}
> >>> +
> >>>    	for (i = startindex; i<   *count; i++) {
> >>>    		vout->buffer_size = *size;
> >>>
> >>> @@ -856,6 +864,14 @@ static int omap_vout_mmap(struct file *file,
> struct
> >> vm_area_struct *vma)
> >>>    				(vma->vm_pgoff<<   PAGE_SHIFT));
> >>>    		return -EINVAL;
> >>>    	}
> >>> +	/* Check the size of the buffer */
> >>> +	if (size>   vout->buffer_size) {
> >>> +		v4l2_err(&vout->vid_dev->v4l2_dev,
> >>> +				"insufficient memory [%lu] [%u]\n",
> >>> +				size, vout->buffer_size);
> >>> +		return -ENOMEM;
> >>> +	}
> >>> +
> >>>    	q->bufs[i]->baddr = vma->vm_start;
> >>>
> >>>    	vma->vm_flags |= VM_RESERVED;
> >>> --
> >>> 1.6.2.4
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-media"
> >> in
> >>> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>
> >
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux