diff options
| author | Hans Verkuil <hans.verkuil@cisco.com> | 2013-12-13 13:13:40 -0300 |
|---|---|---|
| committer | Mauro Carvalho Chehab <m.chehab@samsung.com> | 2014-01-07 07:08:47 -0200 |
| commit | 63faabfd89f4db9862ae2a663193e511419c67eb (patch) | |
| tree | daf06328aa1e9e234cbb481e9a5f5c12a1818ac1 | |
| parent | 4138111a27859dcc56a5592c804dd16bb12a23d1 (diff) | |
[media] vb2: fix race condition between REQBUFS and QBUF/PREPARE_BUF
When preparing a buffer the queue lock is released for a short while
if the memory mode is USERPTR (see __buf_prepare for the details), which
would allow a race with a REQBUFS which can free the buffers. Removing the
buffers from underneath __buf_prepare is obviously a bad idea, so we
check if any of the buffers is in the state PREPARING, and if so we
just return -EAGAIN.
If this happens, then the application does something really strange. The
REQBUFS call can be retried safely, since this situation is transient.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
| -rw-r--r-- | drivers/media/v4l2-core/videobuf2-core.c | 25 |
1 files changed, 23 insertions, 2 deletions
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index ca255da65108..a9a9c6a1c158 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -298,10 +298,28 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) * related information, if no buffers are left return the queue to an * uninitialized state. Might be called even if the queue has already been freed. */ -static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) +static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) { unsigned int buffer; + /* + * Sanity check: when preparing a buffer the queue lock is released for + * a short while (see __buf_prepare for the details), which would allow + * a race with a reqbufs which can call this function. Removing the + * buffers from underneath __buf_prepare is obviously a bad idea, so we + * check if any of the buffers is in the state PREPARING, and if so we + * just return -EAGAIN. + */ + for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; + ++buffer) { + if (q->bufs[buffer] == NULL) + continue; + if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) { + dprintk(1, "reqbufs: preparing buffers, cannot free\n"); + return -EAGAIN; + } + } + /* Call driver-provided cleanup function for each buffer, if provided */ if (q->ops->buf_cleanup) { for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; @@ -326,6 +344,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) if (!q->num_buffers) q->memory = 0; INIT_LIST_HEAD(&q->queued_list); + return 0; } /** @@ -658,7 +677,9 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) return -EBUSY; } - __vb2_queue_free(q, q->num_buffers); + ret = __vb2_queue_free(q, q->num_buffers); + if (ret) + return ret; /* * In case of REQBUFS(0) return immediately without calling |