On Wed, Jul 18, 2018 at 11:29:01AM +0200, Hans Verkuil wrote:
> On 16/07/18 14:49, Sakari Ailus wrote:
> > Hi Hans,
> >
> > On Thu, Jul 05, 2018 at 10:25:19AM +0200, Hans Verkuil wrote:
> >> The vb2_core_qbuf() function didn't check if q->error was set. It is checked in
> >> __buf_prepare(), but that function isn't called if the buffer was already
> >> prepared before with VIDIOC_PREPARE_BUF.
> >>
> >> So check it at the start of vb2_core_qbuf() as well.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >> ---
> >> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> >> index d3501cd604cb..5d7946ec80d8 100644
> >> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> >> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> >> @@ -1484,6 +1484,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
> >> struct vb2_buffer *vb;
> >> int ret;
> >>
> >> + if (q->error) {
> >> + dprintk(1, "fatal error occurred on queue\n");
> >> + return -EIO;
> >> + }
> >> +
> >> vb = q->bufs[index];
> >>
> >> if ((req && q->uses_qbuf) ||
> >
> > How long has this problem existed? It looks like something that should go
> > to the stable branches, too...
>
> It's always been there, but I don't think it is worth backporting. The use of
> VIDIOC_PREPARE_BUF is very rare, let alone the combination with vb2_queue_error().
>
> I came across it while reviewing code.
What's the effect of the missing check? That the user may queue a buffer
when the driver thinks the hardware won't be able to complete it? At least
that doesn't seem like a security issue.
Anyway,
Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
--
Sakari Ailus
e-mail: sakari.ailus@xxxxxx
