Hello,
On Thursday, November 24, 2011 12:27 PM Laurent Pinchart wrote:
> On Wednesday 16 November 2011 20:22:28 Marek Szyprowski wrote:
> > Heuristic that checks if the memory pointer has been changed lacked a check
> > if the pointer was actually provided by the userspace, what allowed one to
> > queue a NULL pointer which was accepted without further checking.
>
> Is that an issue ? If the pointer is NULL get_user_pages() will fail, won't it
> ?
get_user_pages() fails correctly on NULL pointer, but this patch about something
else. VB2 heuristically checks if the user ptr has been changed compared to previous
call to QBUF - if so, it releases old buffer and tries to grab a new one with new
user ptr value.
This heuristic failed if user queued NULL user ptr as a first buffer. All the buffer
internal structures are zeroed on init, so the comparison of NULL pointer with
initial zero value was true. VB2 incorrectly assumed that it can reuse the pointer
from the previous QBUF call what caused a kernel ops a few lines later.
>
> > This patch fixes this issue.
> >
> > Reported-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> > 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 | 3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/media/video/videobuf2-core.c
> > b/drivers/media/video/videobuf2-core.c index ec49fed..24f11ae 100644
> > --- a/drivers/media/video/videobuf2-core.c
> > +++ b/drivers/media/video/videobuf2-core.c
> > @@ -765,7 +765,8 @@ static int __qbuf_userptr(struct vb2_buffer *vb, struct
> > v4l2_buffer *b)
> >
> > for (plane = 0; plane < vb->num_planes; ++plane) {
> > /* Skip the plane if already verified */
> > - if (vb->v4l2_planes[plane].m.userptr == planes[plane].m.userptr
> > + if (vb->v4l2_planes[plane].m.userptr &&
> > + vb->v4l2_planes[plane].m.userptr == planes[plane].m.userptr
> > && vb->v4l2_planes[plane].length == planes[plane].length)
> > continue;
>
> --
> Regards,
>
> Laurent Pinchart
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
