Videobuf2 corner case fixes
Subsystems (2)
v3 cover Videobuf2 corner case fixes
Hi all,
Here's a third version of the set fixing a few videobuf2 corner cases. Most drivers have limits for the size already but not necessarily all of them.
since v2:
- Remove the sanity check from the second patch as suggested by Mauro.
since v1:
- Add a sanity check for alignment in vb2_dma_sg_alloc_compacted.
- Add a comment in __vb2_buf_mem_alloc noting that the size shall be page aligned.
Sakari Ailus (3):
videobuf2-core: Prevent size alignment wrapping buffer size to 0
videobuf2-dma-sg: Prevent size from overflowing videobuf2-core.h: Document the alloc memop size argument as page
aligned
drivers/media/common/videobuf2/videobuf2-core.c | 5 +++++ drivers/media/common/videobuf2/videobuf2-dma-sg.c | 2 +- include/media/videobuf2-core.h | 3 ++- 3 files changed, 8 insertions(+), 2 deletions(-)
Signature · 1 line
2.11.0
PATCH v3 1/3 videobuf2-core: Prevent size alignment wrapping buffer size to 0
PAGE_ALIGN() may wrap the buffer size around to 0. Prevent this by checking that the aligned value is not smaller than the unaligned one.
Note on backporting to stable: the file used to be under drivers/media/v4l2-core, it was moved to the current location after 4.14.
--- drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/media/common/videobuf2/videobuf2-core.c +4 -0
@@ -207,6 +207,10 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)for (plane =0 ; plane < vb->num_planes; ++plane) {unsigned long size =PAGE_ALIGN (vb->planes[plane].length); +/* Did it wrap around? */ +if (size < vb->planes[plane].length) + goto free; + mem_priv =call_ptr_memop (vb, alloc, q->alloc_devs[plane] ? : q->dev, q->dma_attrs, size, q->dma_dir, q->gfp_flags);
Signature · 1 line
2.11.0
PATCH v3 2/3 videobuf2-dma-sg: Prevent size from overflowing
In reply to Sakari Ailusbuf->size is an unsigned long; casting that to int will lead to an overflow if buf->size exceeds INT_MAX.
Fix this by changing the type to unsigned long instead. This is possible as the buf->size is always aligned to PAGE_SIZE, and therefore the size will never have values lesser than 0.
Note on backporting to stable: the file used to be under drivers/media/v4l2-core, it was moved to the current location after 4.14.
--- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/common/videobuf2/videobuf2-dma-sg.c +1 -1
@@ -59,7 +59,7 @@ static int vb2_dma_sg_alloc_compacted(struct vb2_dma_sg_buf *buf, gfp_t gfp_flags) {unsigned int last_page =0 ; -int size = buf->size; +unsigned long size = buf->size;while (size >0 ) {struct page *pages;
Signature · 1 line
2.11.0
PATCH v3 3/3 videobuf2-core.h: Document the alloc memop size argument as page aligned
In reply to Sakari AilusThe size argument of the alloc memop, which allocates buffer memory, is page aligned. Document it as such in the only caller as well as ops documentation.
--- drivers/media/common/videobuf2/videobuf2-core.c | 1 + include/media/videobuf2-core.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)
drivers/media/common/videobuf2/videobuf2-core.c +1 -0
@@ -205,6 +205,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)* NOTE: mmapped areas should be page aligned */ for (plane =0 ; plane < vb->num_planes; ++plane) { +/* Memops alloc requires size to be page aligned. */ unsigned long size =PAGE_ALIGN (vb->planes[plane].length);/* Did it wrap around? */
include/media/videobuf2-core.h +2 -1
@@ -54,7 +54,8 @@ struct vb2_threadio_data;* will then be passed as @buf_priv argument to other ops in this * structure. Additional gfp_flags to use when allocating the * are also passed to this operation. These flags are from the -* gfp_flags field of vb2_queue. +* gfp_flags field of vb2_queue. The size argument to this function +* shall be *page aligned*. * @put: inform the allocator that the buffer will no longer be used; * usually will result in the allocator freeing the buffer (if * no other users of this buffer are present); the @buf_priv
Signature · 1 line
2.11.0