Re: [PATCH] media: vb2: add a check if queued userptr buffer is large enough

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

 



On Wednesday, August 10, 2011 11:08:20 Marek Szyprowski wrote:
> Hello,
> 
> On Wednesday, August 10, 2011 10:46 AM Hans Verkuil wrote:
> 
> > Just one comment:
> > 
> > On Wednesday, August 10, 2011 10:21:28 Marek Szyprowski wrote:
> > > Videobuf2 accepted any userptr buffer without verifying if its size is
> > > large enough to store the video data from the driver. The driver reports
> > > the minimal size of video data once in queue_setup and expects that
> > > videobuf2 provides buffers that match these requirements. This patch
> > > adds the required check.
> > >
> > > Reported-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > > CC: Pawel Osciak <pawel@xxxxxxxxxx>
> > > ---
> > >  drivers/media/video/videobuf2-core.c |   41
> > +++++++++++++++++++--------------
> > >  include/media/videobuf2-core.h       |    1 +
> > >  2 files changed, 25 insertions(+), 17 deletions(-)
> > >
> > 
> > <snip>
> > 
> > > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-
core.h
> > > index f87472a..496d6e5 100644
> > > --- a/include/media/videobuf2-core.h
> > > +++ b/include/media/videobuf2-core.h
> > > @@ -276,6 +276,7 @@ struct vb2_queue {
> > >  	wait_queue_head_t		done_wq;
> > >
> > >  	void				*alloc_ctx[VIDEO_MAX_PLANES];
> > > +	unsigned long			plane_sizes[VIDEO_MAX_PLANES];
> > 
> > Why unsigned long when it is a u32 in struct v4l2_plane_pix_format?
> > 
> > unsigned long is 64 bit on a 64-bit OS, so that seems wasteful to me.
> 
> u32 type should be used in places where the exact size really matters,
> like strictly defined io structures passed to userspace or structures that
> are used for accessing hardware registers directly. For all other cases,
> like temporary storage of some values, the cpu native types should be used.
> Looks at the whole vb2 code - u32 type is not used in any single place.

You can also change unsigned long to unsigned int as that is always 32 bits.

I don't mind one way or another.

Regards,

	Hans

> 
> Best regards
> -- 
> Marek Szyprowski
> Samsung Poland R&D Center
> 
> 
--
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