This is a note to let you know that I've just added the patch titled
media: videobuf2-core: take mmap_lock in vb2_get_unmapped_area()
to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
media-videobuf2-core-take-mmap_lock-in-vb2_get_unmap.patch
and it can be found in the queue-5.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
commit e5e26144a62a3037222e7eb80ffe8ed2d249ca26
Author: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
Date: Wed Dec 7 13:04:34 2022 +0000
media: videobuf2-core: take mmap_lock in vb2_get_unmapped_area()
[ Upstream commit 098e5edc5d048a8df8691fd9fde895af100be42b ]
While vb2_mmap took the mmap_lock mutex, vb2_get_unmapped_area didn't.
Add this.
Also take this opportunity to move the 'q->memory != VB2_MEMORY_MMAP'
check and vb2_fileio_is_active() check into __find_plane_by_offset() so
both vb2_mmap and vb2_get_unmapped_area do the same checks.
Since q->memory is checked while mmap_lock is held, also take that lock
in reqbufs and create_bufs when it is set, and set it back to
MEMORY_UNKNOWN on error.
Fixes: f035eb4e976e ("[media] videobuf2: fix lockdep warning")
Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
Acked-by: Tomasz Figa <tfiga@xxxxxxxxxxxx>
Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 72350343a56a..3bafde87a125 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -787,7 +787,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
num_buffers = max_t(unsigned int, *count, q->min_buffers_needed);
num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME);
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
+ /*
+ * Set this now to ensure that drivers see the correct q->memory value
+ * in the queue_setup op.
+ */
+ mutex_lock(&q->mmap_lock);
q->memory = memory;
+ mutex_unlock(&q->mmap_lock);
/*
* Ask the driver how many buffers and planes per buffer it requires.
@@ -796,22 +802,27 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
plane_sizes, q->alloc_devs);
if (ret)
- return ret;
+ goto error;
/* Check that driver has set sane values */
- if (WARN_ON(!num_planes))
- return -EINVAL;
+ if (WARN_ON(!num_planes)) {
+ ret = -EINVAL;
+ goto error;
+ }
for (i = 0; i < num_planes; i++)
- if (WARN_ON(!plane_sizes[i]))
- return -EINVAL;
+ if (WARN_ON(!plane_sizes[i])) {
+ ret = -EINVAL;
+ goto error;
+ }
/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes, plane_sizes);
if (allocated_buffers == 0) {
dprintk(q, 1, "memory allocation failed\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto error;
}
/*
@@ -852,7 +863,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
if (ret < 0) {
/*
* Note: __vb2_queue_free() will subtract 'allocated_buffers'
- * from q->num_buffers.
+ * from q->num_buffers and it will reset q->memory to
+ * VB2_MEMORY_UNKNOWN.
*/
__vb2_queue_free(q, allocated_buffers);
mutex_unlock(&q->mmap_lock);
@@ -868,6 +880,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
q->waiting_for_buffers = !q->is_output;
return 0;
+
+error:
+ mutex_lock(&q->mmap_lock);
+ q->memory = VB2_MEMORY_UNKNOWN;
+ mutex_unlock(&q->mmap_lock);
+ return ret;
}
EXPORT_SYMBOL_GPL(vb2_core_reqbufs);
@@ -878,6 +896,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
{
unsigned int num_planes = 0, num_buffers, allocated_buffers;
unsigned plane_sizes[VB2_MAX_PLANES] = { };
+ bool no_previous_buffers = !q->num_buffers;
int ret;
if (q->num_buffers == VB2_MAX_FRAME) {
@@ -885,13 +904,19 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
return -ENOBUFS;
}
- if (!q->num_buffers) {
+ if (no_previous_buffers) {
if (q->waiting_in_dqbuf && *count) {
dprintk(q, 1, "another dup()ped fd is waiting for a buffer\n");
return -EBUSY;
}
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
+ /*
+ * Set this now to ensure that drivers see the correct q->memory
+ * value in the queue_setup op.
+ */
+ mutex_lock(&q->mmap_lock);
q->memory = memory;
+ mutex_unlock(&q->mmap_lock);
q->waiting_for_buffers = !q->is_output;
} else {
if (q->memory != memory) {
@@ -914,14 +939,15 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
ret = call_qop(q, queue_setup, q, &num_buffers,
&num_planes, plane_sizes, q->alloc_devs);
if (ret)
- return ret;
+ goto error;
/* Finally, allocate buffers and video memory */
allocated_buffers = __vb2_queue_alloc(q, memory, num_buffers,
num_planes, plane_sizes);
if (allocated_buffers == 0) {
dprintk(q, 1, "memory allocation failed\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto error;
}
/*
@@ -952,7 +978,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
if (ret < 0) {
/*
* Note: __vb2_queue_free() will subtract 'allocated_buffers'
- * from q->num_buffers.
+ * from q->num_buffers and it will reset q->memory to
+ * VB2_MEMORY_UNKNOWN.
*/
__vb2_queue_free(q, allocated_buffers);
mutex_unlock(&q->mmap_lock);
@@ -967,6 +994,14 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
*count = allocated_buffers;
return 0;
+
+error:
+ if (no_previous_buffers) {
+ mutex_lock(&q->mmap_lock);
+ q->memory = VB2_MEMORY_UNKNOWN;
+ mutex_unlock(&q->mmap_lock);
+ }
+ return ret;
}
EXPORT_SYMBOL_GPL(vb2_core_create_bufs);
@@ -2120,6 +2155,22 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
struct vb2_buffer *vb;
unsigned int buffer, plane;
+ /*
+ * Sanity checks to ensure the lock is held, MEMORY_MMAP is
+ * used and fileio isn't active.
+ */
+ lockdep_assert_held(&q->mmap_lock);
+
+ if (q->memory != VB2_MEMORY_MMAP) {
+ dprintk(q, 1, "queue is not currently set up for mmap\n");
+ return -EINVAL;
+ }
+
+ if (vb2_fileio_is_active(q)) {
+ dprintk(q, 1, "file io in progress\n");
+ return -EBUSY;
+ }
+
/*
* Go over all buffers and their planes, comparing the given offset
* with an offset assigned to each plane. If a match is found,
@@ -2219,11 +2270,6 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
int ret;
unsigned long length;
- if (q->memory != VB2_MEMORY_MMAP) {
- dprintk(q, 1, "queue is not currently set up for mmap\n");
- return -EINVAL;
- }
-
/*
* Check memory area access mode.
*/
@@ -2245,14 +2291,9 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
mutex_lock(&q->mmap_lock);
- if (vb2_fileio_is_active(q)) {
- dprintk(q, 1, "mmap: file io in progress\n");
- ret = -EBUSY;
- goto unlock;
- }
-
/*
- * Find the plane corresponding to the offset passed by userspace.
+ * Find the plane corresponding to the offset passed by userspace. This
+ * will return an error if not MEMORY_MMAP or file I/O is in progress.
*/
ret = __find_plane_by_offset(q, off, &buffer, &plane);
if (ret)
@@ -2305,22 +2346,25 @@ unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
void *vaddr;
int ret;
- if (q->memory != VB2_MEMORY_MMAP) {
- dprintk(q, 1, "queue is not currently set up for mmap\n");
- return -EINVAL;
- }
+ mutex_lock(&q->mmap_lock);
/*
- * Find the plane corresponding to the offset passed by userspace.
+ * Find the plane corresponding to the offset passed by userspace. This
+ * will return an error if not MEMORY_MMAP or file I/O is in progress.
*/
ret = __find_plane_by_offset(q, off, &buffer, &plane);
if (ret)
- return ret;
+ goto unlock;
vb = q->bufs[buffer];
vaddr = vb2_plane_vaddr(vb, plane);
+ mutex_unlock(&q->mmap_lock);
return vaddr ? (unsigned long)vaddr : -EINVAL;
+
+unlock:
+ mutex_unlock(&q->mmap_lock);
+ return ret;
}
EXPORT_SYMBOL_GPL(vb2_get_unmapped_area);
#endif
