* [PATCH v2 0/3] Videobuf2 corner case fixes
@ 2019-01-08 8:58 Sakari Ailus
2019-01-08 8:58 ` [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 Sakari Ailus
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Sakari Ailus @ 2019-01-08 8:58 UTC (permalink / raw)
To: linux-media; +Cc: hverkuil, mchehab, laurent.pinchart
Hi all,
Here's a second 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 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 | 5 ++++-
include/media/videobuf2-core.h | 3 ++-
3 files changed, 11 insertions(+), 2 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 8:58 [PATCH v2 0/3] Videobuf2 corner case fixes Sakari Ailus @ 2019-01-08 8:58 ` Sakari Ailus 2019-01-08 12:52 ` Mauro Carvalho Chehab 2019-01-08 8:58 ` [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing Sakari Ailus 2019-01-08 8:58 ` [PATCH v2 3/3] videobuf2-core.h: Document the alloc memop size argument as page aligned Sakari Ailus 2 siblings, 1 reply; 19+ messages in thread From: Sakari Ailus @ 2019-01-08 8:58 UTC (permalink / raw) To: linux-media; +Cc: hverkuil, mchehab, laurent.pinchart 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. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: stable@vger.kernel.org Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 0ca81d495bda..0234ddbfa4de 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -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); -- 2.11.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 8:58 ` [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 Sakari Ailus @ 2019-01-08 12:52 ` Mauro Carvalho Chehab 2019-01-08 12:59 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-08 12:52 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil, laurent.pinchart Em Tue, 8 Jan 2019 10:58:34 +0200 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > 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. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: stable@vger.kernel.org > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index 0ca81d495bda..0234ddbfa4de 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -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; > + Sorry, but I can't see how this could ever happen (except for a very serious bug at the compiler or at the hardware). See, the definition at PAGE_ALIGN is (from mm.h): #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) and the macro it uses come from kernel.h: #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) .. #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) So, this: size = PAGE_ALIGN(length); (assuming PAGE_SIZE= 0x1000) becomes: size = (length + 0x0fff) & ~0xfff; so, size will *always* be >= length. Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 12:52 ` Mauro Carvalho Chehab @ 2019-01-08 12:59 ` Mauro Carvalho Chehab 2019-01-08 13:01 ` Mauro Carvalho Chehab ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-08 12:59 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil, laurent.pinchart Em Tue, 8 Jan 2019 10:52:12 -0200 Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > Em Tue, 8 Jan 2019 10:58:34 +0200 > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > 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. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Cc: stable@vger.kernel.org > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > --- > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > > index 0ca81d495bda..0234ddbfa4de 100644 > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > @@ -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; > > + > > Sorry, but I can't see how this could ever happen (except for a very serious > bug at the compiler or at the hardware). > > See, the definition at PAGE_ALIGN is (from mm.h): > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > and the macro it uses come from kernel.h: > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > .. > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > So, this: > size = PAGE_ALIGN(length); > > (assuming PAGE_SIZE= 0x1000) > > becomes: > > size = (length + 0x0fff) & ~0xfff; > > so, size will *always* be >= length. Hmm... after looking at patch 2, now I understand what's your concern... If someone indeed uses length = INT_MAX, size will indeed be zero. Please adjust the description accordingly, as it doesn't reflect that. Btw, in this particular case, I would use a WARN_ON(), as this is something that indicates not only a driver bug (as the driver is letting someone to request a buffer a way too big), but probably also an attempt from a hacker to try to crack the system. Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 12:59 ` Mauro Carvalho Chehab @ 2019-01-08 13:01 ` Mauro Carvalho Chehab 2019-01-08 13:38 ` Sakari Ailus 2019-01-08 13:40 ` Sakari Ailus 2 siblings, 0 replies; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-08 13:01 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil, laurent.pinchart Em Tue, 8 Jan 2019 10:59:55 -0200 Mauro Carvalho Chehab <mchehab+samsung@kernel.org> escreveu: > Em Tue, 8 Jan 2019 10:52:12 -0200 > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > 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. > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Cc: stable@vger.kernel.org > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > --- > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > > > index 0ca81d495bda..0234ddbfa4de 100644 > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > @@ -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; > > > + > > > > Sorry, but I can't see how this could ever happen (except for a very serious > > bug at the compiler or at the hardware). > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > and the macro it uses come from kernel.h: > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > .. > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > So, this: > > size = PAGE_ALIGN(length); > > > > (assuming PAGE_SIZE= 0x1000) > > > > becomes: > > > > size = (length + 0x0fff) & ~0xfff; > > > > so, size will *always* be >= length. > > Hmm... after looking at patch 2, now I understand what's your concern... > > If someone indeed uses length = INT_MAX, size will indeed be zero. In time, I meant to say UINT_MAX. > > Please adjust the description accordingly, as it doesn't reflect > that. > > Btw, in this particular case, I would use a WARN_ON(), as this is > something that indicates not only a driver bug (as the driver is > letting someone to request a buffer a way too big), but probably > also an attempt from a hacker to try to crack the system. > > Thanks, > Mauro Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 12:59 ` Mauro Carvalho Chehab 2019-01-08 13:01 ` Mauro Carvalho Chehab @ 2019-01-08 13:38 ` Sakari Ailus 2019-01-08 14:23 ` Mauro Carvalho Chehab 2019-01-08 13:40 ` Sakari Ailus 2 siblings, 1 reply; 19+ messages in thread From: Sakari Ailus @ 2019-01-08 13:38 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, hverkuil, laurent.pinchart Hi Mauro, Thanks for the review. On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > Em Tue, 8 Jan 2019 10:52:12 -0200 > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > 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. > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Cc: stable@vger.kernel.org > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > --- > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > > > index 0ca81d495bda..0234ddbfa4de 100644 > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > @@ -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; > > > + > > > > Sorry, but I can't see how this could ever happen (except for a very serious > > bug at the compiler or at the hardware). > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > and the macro it uses come from kernel.h: > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > .. > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > So, this: > > size = PAGE_ALIGN(length); > > > > (assuming PAGE_SIZE= 0x1000) > > > > becomes: > > > > size = (length + 0x0fff) & ~0xfff; > > > > so, size will *always* be >= length. > > Hmm... after looking at patch 2, now I understand what's your concern... > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > Please adjust the description accordingly, as it doesn't reflect > that. > > Btw, in this particular case, I would use a WARN_ON(), as this is > something that indicates not only a driver bug (as the driver is > letting someone to request a buffer a way too big), but probably What's the maximum size a driver should allow? I guess this could be seen be a failure from the driver's part to limit the size of the buffer, but it's not trivial either to define that. Hardware typically has maximum dimensions it can support, but the user may want to add padding at the end of the lines. Perhaps a helper macro could be used for this purpose: most likely there's no need to be more padding than there's image data per line. If that turns out to be too restrictive, the macro could be changed. That's probably unlikely, admittedly. For some hardware these numbers could still be more than a 32-bit unsigned integer can hold, so the check is still needed. > also an attempt from a hacker to try to crack the system. This could be also v4l2-compliance setting the length field to -1. A warning is worth it only if there's good chance there's e.g. a kernel bug involved. -- Kind regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 13:38 ` Sakari Ailus @ 2019-01-08 14:23 ` Mauro Carvalho Chehab 2019-01-09 8:41 ` Sakari Ailus 0 siblings, 1 reply; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-08 14:23 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil, laurent.pinchart Em Tue, 8 Jan 2019 15:38:32 +0200 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > Hi Mauro, > > Thanks for the review. > > On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > > Em Tue, 8 Jan 2019 10:52:12 -0200 > > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > > > 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. > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Cc: stable@vger.kernel.org > > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > > --- > > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > > > > index 0ca81d495bda..0234ddbfa4de 100644 > > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > > @@ -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; > > > > + > > > > > > Sorry, but I can't see how this could ever happen (except for a very serious > > > bug at the compiler or at the hardware). > > > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > > > and the macro it uses come from kernel.h: > > > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) > > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > > .. > > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > > > So, this: > > > size = PAGE_ALIGN(length); > > > > > > (assuming PAGE_SIZE= 0x1000) > > > > > > becomes: > > > > > > size = (length + 0x0fff) & ~0xfff; > > > > > > so, size will *always* be >= length. > > > > Hmm... after looking at patch 2, now I understand what's your concern... > > > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > > > Please adjust the description accordingly, as it doesn't reflect > > that. > > > > Btw, in this particular case, I would use a WARN_ON(), as this is > > something that indicates not only a driver bug (as the driver is > > letting someone to request a buffer a way too big), but probably > > What's the maximum size a driver should allow? I guess this could be seen > be a failure from the driver's part to limit the size of the buffer, but > it's not trivial either to define that. > > Hardware typically has maximum dimensions it can support, but the user may > want to add padding at the end of the lines. Perhaps a helper macro could > be used for this purpose: most likely there's no need to be more padding > than there's image data per line. If that turns out to be too restrictive, > the macro could be changed. That's probably unlikely, admittedly. > > For some hardware these numbers could still be more than a 32-bit unsigned > integer can hold, so the check is still needed. I guess that, by changing from "int" to "unsigned long", we ensure that the number should be big enough to be able to represent the maximum allocation size. On Linux, sizeof(long) is usually assumed to be sizeof(void *). Such assumption is used, for example, when we pass a structure pointer to ioctl's, instead of passing a long integer. I mean, on a 64 bits system, long has 64 bits. AFAIKT, even the latest Xeon CPUs, the address space is lower than 64 bits. So, if one tries to allocate a memory with sizeof(ULONG_MAX), this will fail with ENOMEM. On any (true) 32 bits system, the physical address is to 32 bits. So, if one tries to allocate a memory with ULONG_MAX, this should also fail, as there won't be memory for anything else. There are some special cases, like X86_PAE (and ARM_LPAE). There, the physical address space is 64 bits, but instruction set is the 32 bits one. Yet, I'm almost sure that (at least on x86) a single memory block there can't be bigger than 32 bits. What I'm trying to say is that I strongly suspect that we won't have any cases where someone using would need a buffer with more than 32 bits size on a non-64 architecture. > > > also an attempt from a hacker to try to crack the system. > > This could be also v4l2-compliance setting the length field to -1. A > warning is worth it only if there's good chance there's e.g. a kernel bug > involved. > Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 14:23 ` Mauro Carvalho Chehab @ 2019-01-09 8:41 ` Sakari Ailus 0 siblings, 0 replies; 19+ messages in thread From: Sakari Ailus @ 2019-01-09 8:41 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, hverkuil, laurent.pinchart On Tue, Jan 08, 2019 at 12:23:49PM -0200, Mauro Carvalho Chehab wrote: > Em Tue, 8 Jan 2019 15:38:32 +0200 > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > Hi Mauro, > > > > Thanks for the review. > > > > On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > > > Em Tue, 8 Jan 2019 10:52:12 -0200 > > > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > > > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > > > > > 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. > > > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > Cc: stable@vger.kernel.org > > > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > > > --- > > > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > > > > > index 0ca81d495bda..0234ddbfa4de 100644 > > > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > > > @@ -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; > > > > > + > > > > > > > > Sorry, but I can't see how this could ever happen (except for a very serious > > > > bug at the compiler or at the hardware). > > > > > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > > > > > and the macro it uses come from kernel.h: > > > > > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) > > > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > > > .. > > > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > > > > > So, this: > > > > size = PAGE_ALIGN(length); > > > > > > > > (assuming PAGE_SIZE= 0x1000) > > > > > > > > becomes: > > > > > > > > size = (length + 0x0fff) & ~0xfff; > > > > > > > > so, size will *always* be >= length. > > > > > > Hmm... after looking at patch 2, now I understand what's your concern... > > > > > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > > > > > Please adjust the description accordingly, as it doesn't reflect > > > that. > > > > > > Btw, in this particular case, I would use a WARN_ON(), as this is > > > something that indicates not only a driver bug (as the driver is > > > letting someone to request a buffer a way too big), but probably > > > > What's the maximum size a driver should allow? I guess this could be seen > > be a failure from the driver's part to limit the size of the buffer, but > > it's not trivial either to define that. > > > > Hardware typically has maximum dimensions it can support, but the user may > > want to add padding at the end of the lines. Perhaps a helper macro could > > be used for this purpose: most likely there's no need to be more padding > > than there's image data per line. If that turns out to be too restrictive, > > the macro could be changed. That's probably unlikely, admittedly. > > > > For some hardware these numbers could still be more than a 32-bit unsigned > > integer can hold, so the check is still needed. > > I guess that, by changing from "int" to "unsigned long", we ensure that the > number should be big enough to be able to represent the maximum allocation > size. > > On Linux, sizeof(long) is usually assumed to be sizeof(void *). Such > assumption is used, for example, when we pass a structure pointer to > ioctl's, instead of passing a long integer. > > I mean, on a 64 bits system, long has 64 bits. AFAIKT, even the latest > Xeon CPUs, the address space is lower than 64 bits. So, if one tries to > allocate a memory with sizeof(ULONG_MAX), this will fail with ENOMEM. > > On any (true) 32 bits system, the physical address is to 32 bits. > So, if one tries to allocate a memory with ULONG_MAX, this should > also fail, as there won't be memory for anything else. > > There are some special cases, like X86_PAE (and ARM_LPAE). There, the > physical address space is 64 bits, but instruction set is the 32 bits one. > Yet, I'm almost sure that (at least on x86) a single memory block there > can't be bigger than 32 bits. > > What I'm trying to say is that I strongly suspect that we won't have > any cases where someone using would need a buffer with more than > 32 bits size on a non-64 architecture. I agree; also the length field in struct v4l2_buffer is a __u32 so that limits the value range for size as well. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 12:59 ` Mauro Carvalho Chehab 2019-01-08 13:01 ` Mauro Carvalho Chehab 2019-01-08 13:38 ` Sakari Ailus @ 2019-01-08 13:40 ` Sakari Ailus 2019-01-08 14:30 ` Mauro Carvalho Chehab 2 siblings, 1 reply; 19+ messages in thread From: Sakari Ailus @ 2019-01-08 13:40 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, hverkuil, laurent.pinchart On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > Em Tue, 8 Jan 2019 10:52:12 -0200 > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > 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. > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Cc: stable@vger.kernel.org > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > --- > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > > > index 0ca81d495bda..0234ddbfa4de 100644 > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > @@ -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; > > > + > > > > Sorry, but I can't see how this could ever happen (except for a very serious > > bug at the compiler or at the hardware). > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > and the macro it uses come from kernel.h: > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > .. > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > So, this: > > size = PAGE_ALIGN(length); > > > > (assuming PAGE_SIZE= 0x1000) > > > > becomes: > > > > size = (length + 0x0fff) & ~0xfff; > > > > so, size will *always* be >= length. > > Hmm... after looking at patch 2, now I understand what's your concern... > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > Please adjust the description accordingly, as it doesn't reflect > that. How about: PAGE_ALIGN() may wrap the buffer length around to 0 if the value to be aligned is close to the top of the value range of the type. Prevent this by checking that the aligned value is not smaller than the unaligned one. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 13:40 ` Sakari Ailus @ 2019-01-08 14:30 ` Mauro Carvalho Chehab 2019-01-08 16:05 ` Laurent Pinchart 0 siblings, 1 reply; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-08 14:30 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil, laurent.pinchart Em Tue, 8 Jan 2019 15:40:47 +0200 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > > Em Tue, 8 Jan 2019 10:52:12 -0200 > > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > > > 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. > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Cc: stable@vger.kernel.org > > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > > --- > > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > > > > index 0ca81d495bda..0234ddbfa4de 100644 > > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > > @@ -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; > > > > + > > > > > > Sorry, but I can't see how this could ever happen (except for a very serious > > > bug at the compiler or at the hardware). > > > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > > > and the macro it uses come from kernel.h: > > > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) > > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > > .. > > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > > > So, this: > > > size = PAGE_ALIGN(length); > > > > > > (assuming PAGE_SIZE= 0x1000) > > > > > > becomes: > > > > > > size = (length + 0x0fff) & ~0xfff; > > > > > > so, size will *always* be >= length. > > > > Hmm... after looking at patch 2, now I understand what's your concern... > > > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > > > Please adjust the description accordingly, as it doesn't reflect > > that. > > How about: > > PAGE_ALIGN() may wrap the buffer length around to 0 if the value to be > aligned is close to the top of the value range of the type. Prevent this by > checking that the aligned value is not smaller than the unaligned one. I would be a way more clear, as this is there to prevent a single special case: length == ULEN_MAX. Something like: If one tried to allocate a buffer with sizeof(ULEN_MAX), this will cause an overflow at PAGE_ALIGN(), making it return zero as the size of the buffer, causing the code to fail. I would even let it clearer at the code itself. So, instead of the hunk you proposed, I would do: unsigned long size = vb->planes[plane].length; /* Prevent PAGE_ALIGN overflow */ if (WARN_ON(size == ULONG_MAX)) goto free; size = PAGE_ALIGN(vb->planes[plane].length); Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 14:30 ` Mauro Carvalho Chehab @ 2019-01-08 16:05 ` Laurent Pinchart 2019-01-09 12:13 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 19+ messages in thread From: Laurent Pinchart @ 2019-01-08 16:05 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: Sakari Ailus, linux-media, hverkuil On Tuesday, 8 January 2019 16:30:22 EET Mauro Carvalho Chehab wrote: > Em Tue, 8 Jan 2019 15:40:47 +0200 > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > > > Em Tue, 8 Jan 2019 10:52:12 -0200 > > > > > > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > > > > > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > 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. > > > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > Cc: stable@vger.kernel.org > > > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > > > --- > > > > > > > > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > > > > > b/drivers/media/common/videobuf2/videobuf2-core.c index > > > > > 0ca81d495bda..0234ddbfa4de 100644 > > > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > > > @@ -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; > > > > > + > > > > > > > > Sorry, but I can't see how this could ever happen (except for a very > > > > serious bug at the compiler or at the hardware). > > > > > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > > > > > and the macro it uses come from kernel.h: > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x)) (a) - > > > > 1) > > > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > > > .. > > > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > > > > > So, this: > > > > size = PAGE_ALIGN(length); > > > > > > > > (assuming PAGE_SIZE= 0x1000) > > > > > > > > becomes: > > > > size = (length + 0x0fff) & ~0xfff; > > > > > > > > so, size will *always* be >= length. > > > > > > Hmm... after looking at patch 2, now I understand what's your concern... > > > > > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > > > > > Please adjust the description accordingly, as it doesn't reflect > > > that. > > > > How about: > > > > PAGE_ALIGN() may wrap the buffer length around to 0 if the value to be > > aligned is close to the top of the value range of the type. Prevent this > > by > > checking that the aligned value is not smaller than the unaligned one. > > I would be a way more clear, as this is there to prevent a single > special case: length == ULEN_MAX. Something like: > > If one tried to allocate a buffer with sizeof(ULEN_MAX), this will cause > an overflow at PAGE_ALIGN(), making it return zero as the size of the > buffer, causing the code to fail. > > I would even let it clearer at the code itself. So, instead of the > hunk you proposed, I would do: > > unsigned long size = vb->planes[plane].length; > > /* Prevent PAGE_ALIGN overflow */ > if (WARN_ON(size == ULONG_MAX)) > goto free; ULONG_MAX - PAGE_SIZE + 2 to ULONG_MAX would all cause the same issue. > > size = PAGE_ALIGN(vb->planes[plane].length); -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-08 16:05 ` Laurent Pinchart @ 2019-01-09 12:13 ` Mauro Carvalho Chehab 2019-01-09 13:56 ` Sakari Ailus 0 siblings, 1 reply; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-09 12:13 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Sakari Ailus, linux-media, hverkuil Em Tue, 08 Jan 2019 18:05:57 +0200 Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > On Tuesday, 8 January 2019 16:30:22 EET Mauro Carvalho Chehab wrote: > > Em Tue, 8 Jan 2019 15:40:47 +0200 > > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > > > > Em Tue, 8 Jan 2019 10:52:12 -0200 > > > > > > > > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > > > > > > > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > > 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. > > > > > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > Cc: stable@vger.kernel.org > > > > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > > > > --- > > > > > > > > > > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > > > > > > b/drivers/media/common/videobuf2/videobuf2-core.c index > > > > > > 0ca81d495bda..0234ddbfa4de 100644 > > > > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > > > > @@ -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; > > > > > > + > > > > > > > > > > Sorry, but I can't see how this could ever happen (except for a very > > > > > serious bug at the compiler or at the hardware). > > > > > > > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > > > > > > > and the macro it uses come from kernel.h: > > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x)) > (a) - > > > > > 1) > > > > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > > > > .. > > > > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > > > > > > > So, this: > > > > > size = PAGE_ALIGN(length); > > > > > > > > > > (assuming PAGE_SIZE= 0x1000) > > > > > > > > > > becomes: > > > > > size = (length + 0x0fff) & ~0xfff; > > > > > > > > > > so, size will *always* be >= length. > > > > > > > > Hmm... after looking at patch 2, now I understand what's your concern... > > > > > > > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > > > > > > > Please adjust the description accordingly, as it doesn't reflect > > > > that. > > > > > > How about: > > > > > > PAGE_ALIGN() may wrap the buffer length around to 0 if the value to be > > > aligned is close to the top of the value range of the type. Prevent this > > > by > > > checking that the aligned value is not smaller than the unaligned one. > > > > I would be a way more clear, as this is there to prevent a single > > special case: length == ULEN_MAX. Something like: > > > > If one tried to allocate a buffer with sizeof(ULEN_MAX), this will cause > > an overflow at PAGE_ALIGN(), making it return zero as the size of the > > buffer, causing the code to fail. > > > > I would even let it clearer at the code itself. So, instead of the > > hunk you proposed, I would do: > > > > unsigned long size = vb->planes[plane].length; > > > > /* Prevent PAGE_ALIGN overflow */ > > if (WARN_ON(size == ULONG_MAX)) > > goto free; > > ULONG_MAX - PAGE_SIZE + 2 to ULONG_MAX would all cause the same issue. True. The actual check should be: if (WARN_ON(size > ULONG_MAX - PAGE_SIZE + 1)) Not that the original proposal of checking after the overflow is wrong, but, IMHO, something linking the size to ULONG_MAX makes clearer about what kind of issue the code is meant to solve. A good comment before that would work fine. Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 2019-01-09 12:13 ` Mauro Carvalho Chehab @ 2019-01-09 13:56 ` Sakari Ailus 0 siblings, 0 replies; 19+ messages in thread From: Sakari Ailus @ 2019-01-09 13:56 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: Laurent Pinchart, linux-media, hverkuil Hi Mauro, On Wed, Jan 09, 2019 at 10:13:42AM -0200, Mauro Carvalho Chehab wrote: > Em Tue, 08 Jan 2019 18:05:57 +0200 > Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > > > On Tuesday, 8 January 2019 16:30:22 EET Mauro Carvalho Chehab wrote: > > > Em Tue, 8 Jan 2019 15:40:47 +0200 > > > > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > On Tue, Jan 08, 2019 at 10:59:55AM -0200, Mauro Carvalho Chehab wrote: > > > > > Em Tue, 8 Jan 2019 10:52:12 -0200 > > > > > > > > > > Mauro Carvalho Chehab <mchehab@kernel.org> escreveu: > > > > > > Em Tue, 8 Jan 2019 10:58:34 +0200 > > > > > > > > > > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > > > 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. > > > > > > > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > Cc: stable@vger.kernel.org > > > > > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > > > > > --- > > > > > > > > > > > > > > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > > > > > > > b/drivers/media/common/videobuf2/videobuf2-core.c index > > > > > > > 0ca81d495bda..0234ddbfa4de 100644 > > > > > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > > > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > > > > > @@ -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; > > > > > > > + > > > > > > > > > > > > Sorry, but I can't see how this could ever happen (except for a very > > > > > > serious bug at the compiler or at the hardware). > > > > > > > > > > > > See, the definition at PAGE_ALIGN is (from mm.h): > > > > > > #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) > > > > > > > > > > > > and the macro it uses come from kernel.h: > > > > > > #define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x)) > > (a) - > > > > > > 1) > > > > > > #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) > > > > > > .. > > > > > > #define ALIGN(x, a) __ALIGN_KERNEL((x), (a)) > > > > > > > > > > > > So, this: > > > > > > size = PAGE_ALIGN(length); > > > > > > > > > > > > (assuming PAGE_SIZE= 0x1000) > > > > > > > > > > > > becomes: > > > > > > size = (length + 0x0fff) & ~0xfff; > > > > > > > > > > > > so, size will *always* be >= length. > > > > > > > > > > Hmm... after looking at patch 2, now I understand what's your concern... > > > > > > > > > > If someone indeed uses length = INT_MAX, size will indeed be zero. > > > > > > > > > > Please adjust the description accordingly, as it doesn't reflect > > > > > that. > > > > > > > > How about: > > > > > > > > PAGE_ALIGN() may wrap the buffer length around to 0 if the value to be > > > > aligned is close to the top of the value range of the type. Prevent this > > > > by > > > > checking that the aligned value is not smaller than the unaligned one. > > > > > > I would be a way more clear, as this is there to prevent a single > > > special case: length == ULEN_MAX. Something like: > > > > > > If one tried to allocate a buffer with sizeof(ULEN_MAX), this will cause > > > an overflow at PAGE_ALIGN(), making it return zero as the size of the > > > buffer, causing the code to fail. > > > > > > I would even let it clearer at the code itself. So, instead of the > > > hunk you proposed, I would do: > > > > > > unsigned long size = vb->planes[plane].length; > > > > > > /* Prevent PAGE_ALIGN overflow */ > > > if (WARN_ON(size == ULONG_MAX)) > > > goto free; > > > > ULONG_MAX - PAGE_SIZE + 2 to ULONG_MAX would all cause the same issue. > > True. The actual check should be: > > if (WARN_ON(size > ULONG_MAX - PAGE_SIZE + 1)) That is also wrong: aligning ULONG_MAX - PAGE_SIZE + 1 to page is 0, too. > > Not that the original proposal of checking after the overflow is wrong, but, > IMHO, something linking the size to ULONG_MAX makes clearer about what kind > of issue the code is meant to solve. A good comment before that would work > fine. I find the original test more simple to grasp: it is independent of how the alignment is done, to PAGE_SIZE or to something else. It was also right from the start unlike the two other checks that have been proposed so far. Therefore my preference to stick to that. The slight downside is that it is not apparent from the problem is related to aligning to page, but that is mitigated by the comment added by this patch. This property of PAGE_ALIGN() is not usually relevant as typically it's not used to align addresses on the last page (of the value range). -- Kind regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing 2019-01-08 8:58 [PATCH v2 0/3] Videobuf2 corner case fixes Sakari Ailus 2019-01-08 8:58 ` [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 Sakari Ailus @ 2019-01-08 8:58 ` Sakari Ailus 2019-01-08 13:09 ` Mauro Carvalho Chehab 2019-01-08 8:58 ` [PATCH v2 3/3] videobuf2-core.h: Document the alloc memop size argument as page aligned Sakari Ailus 2 siblings, 1 reply; 19+ messages in thread From: Sakari Ailus @ 2019-01-08 8:58 UTC (permalink / raw) To: linux-media; +Cc: hverkuil, mchehab, laurent.pinchart buf->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. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: stable@vger.kernel.org Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c index 015e737095cd..5fdb8d7051f6 100644 --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c @@ -59,7 +59,10 @@ 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; + + if (WARN_ON(size & ~PAGE_MASK)) + return -ENOMEM; while (size > 0) { struct page *pages; -- 2.11.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing 2019-01-08 8:58 ` [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing Sakari Ailus @ 2019-01-08 13:09 ` Mauro Carvalho Chehab 2019-01-08 13:29 ` Sakari Ailus 0 siblings, 1 reply; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-08 13:09 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil, laurent.pinchart Em Tue, 8 Jan 2019 10:58:35 +0200 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > buf->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. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: stable@vger.kernel.org > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 015e737095cd..5fdb8d7051f6 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -59,7 +59,10 @@ 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; OK. > + > + if (WARN_ON(size & ~PAGE_MASK)) > + return -ENOMEM; Hmm... why do we need a warn on here? This is called by this code: static int __vb2_buf_mem_alloc(struct vb2_buffer *vb) { struct vb2_queue *q = vb->vb2_queue; void *mem_priv; int plane; int ret = -ENOMEM; /* * Allocate memory for all planes in this buffer * NOTE: mmapped areas should be page aligned */ for (plane = 0; plane < vb->num_planes; ++plane) { unsigned long size = PAGE_ALIGN(vb->planes[plane].length); mem_priv = call_ptr_memop(vb, alloc, q->alloc_devs[plane] ? : q->dev, q->dma_attrs, size, q->dma_dir, q->gfp_flags); With already insures that size is page aligned. Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing 2019-01-08 13:09 ` Mauro Carvalho Chehab @ 2019-01-08 13:29 ` Sakari Ailus 2019-01-08 13:44 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 19+ messages in thread From: Sakari Ailus @ 2019-01-08 13:29 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, hverkuil, laurent.pinchart On Tue, Jan 08, 2019 at 11:09:42AM -0200, Mauro Carvalho Chehab wrote: > Em Tue, 8 Jan 2019 10:58:35 +0200 > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > buf->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. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Cc: stable@vger.kernel.org > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > --- > > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > index 015e737095cd..5fdb8d7051f6 100644 > > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > @@ -59,7 +59,10 @@ 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; > > OK. > > > + > > + if (WARN_ON(size & ~PAGE_MASK)) > > + return -ENOMEM; > > Hmm... why do we need a warn on here? This is called by this code: This was suggested as a sanity check in review of v1 of the set. Supposing that someone once removed that alignment, things would go rather completely haywire. There would probably be lots of other troubles as well but this one would probably corrupt system memory (at least). > > static int __vb2_buf_mem_alloc(struct vb2_buffer *vb) > { > struct vb2_queue *q = vb->vb2_queue; > void *mem_priv; > int plane; > int ret = -ENOMEM; > > /* > * Allocate memory for all planes in this buffer > * NOTE: mmapped areas should be page aligned > */ > for (plane = 0; plane < vb->num_planes; ++plane) { > unsigned long size = PAGE_ALIGN(vb->planes[plane].length); > > mem_priv = call_ptr_memop(vb, alloc, > q->alloc_devs[plane] ? : q->dev, > q->dma_attrs, size, q->dma_dir, q->gfp_flags); > > With already insures that size is page aligned. > > Thanks, > Mauro -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing 2019-01-08 13:29 ` Sakari Ailus @ 2019-01-08 13:44 ` Mauro Carvalho Chehab 2019-01-08 13:57 ` Sakari Ailus 0 siblings, 1 reply; 19+ messages in thread From: Mauro Carvalho Chehab @ 2019-01-08 13:44 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil, laurent.pinchart Em Tue, 8 Jan 2019 15:29:26 +0200 Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > On Tue, Jan 08, 2019 at 11:09:42AM -0200, Mauro Carvalho Chehab wrote: > > Em Tue, 8 Jan 2019 10:58:35 +0200 > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > buf->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. > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > Cc: stable@vger.kernel.org > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > --- > > > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > > index 015e737095cd..5fdb8d7051f6 100644 > > > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > > @@ -59,7 +59,10 @@ 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; > > > > OK. > > > > > + > > > + if (WARN_ON(size & ~PAGE_MASK)) > > > + return -ENOMEM; > > > > Hmm... why do we need a warn on here? This is called by this code: > > This was suggested as a sanity check in review of v1 of the set. > > Supposing that someone once removed that alignment, things would go rather > completely haywire. There would probably be lots of other troubles as well > but this one would probably corrupt system memory (at least). Well, patch 3 prevents that. See: this is not like something that driver developers can mess with that, as the only place where the .alloc() ops is called is by the VB2 core, and it already ensures page alignment. If one would ever try to remove PAGE_ALIGN() from vb2 core, we'll nack it, as we know that such change will break things. Thanks, Mauro ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing 2019-01-08 13:44 ` Mauro Carvalho Chehab @ 2019-01-08 13:57 ` Sakari Ailus 0 siblings, 0 replies; 19+ messages in thread From: Sakari Ailus @ 2019-01-08 13:57 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, hverkuil, laurent.pinchart On Tue, Jan 08, 2019 at 11:44:01AM -0200, Mauro Carvalho Chehab wrote: > Em Tue, 8 Jan 2019 15:29:26 +0200 > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > On Tue, Jan 08, 2019 at 11:09:42AM -0200, Mauro Carvalho Chehab wrote: > > > Em Tue, 8 Jan 2019 10:58:35 +0200 > > > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu: > > > > > > > buf->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. > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > Cc: stable@vger.kernel.org > > > > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > > > --- > > > > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > > > index 015e737095cd..5fdb8d7051f6 100644 > > > > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > > > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > > > > @@ -59,7 +59,10 @@ 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; > > > > > > OK. > > > > > > > + > > > > + if (WARN_ON(size & ~PAGE_MASK)) > > > > + return -ENOMEM; > > > > > > Hmm... why do we need a warn on here? This is called by this code: > > > > This was suggested as a sanity check in review of v1 of the set. > > > > Supposing that someone once removed that alignment, things would go rather > > completely haywire. There would probably be lots of other troubles as well > > but this one would probably corrupt system memory (at least). > > Well, patch 3 prevents that. See: this is not like something that driver > developers can mess with that, as the only place where the .alloc() ops > is called is by the VB2 core, and it already ensures page alignment. > > If one would ever try to remove PAGE_ALIGN() from vb2 core, we'll nack it, > as we know that such change will break things. Indeed. I'm certainly fine with dropping the sanity check; I think there are enough warnings elsewhere plus common sense to avoid making such a change. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 3/3] videobuf2-core.h: Document the alloc memop size argument as page aligned 2019-01-08 8:58 [PATCH v2 0/3] Videobuf2 corner case fixes Sakari Ailus 2019-01-08 8:58 ` [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 Sakari Ailus 2019-01-08 8:58 ` [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing Sakari Ailus @ 2019-01-08 8:58 ` Sakari Ailus 2 siblings, 0 replies; 19+ messages in thread From: Sakari Ailus @ 2019-01-08 8:58 UTC (permalink / raw) To: linux-media; +Cc: hverkuil, mchehab, laurent.pinchart The 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. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/media/common/videobuf2/videobuf2-core.c | 1 + include/media/videobuf2-core.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 0234ddbfa4de..64ecb9a65a47 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -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? */ diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index e86981d615ae..68b9fe660e4f 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -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 -- 2.11.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-01-09 13:56 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-08 8:58 [PATCH v2 0/3] Videobuf2 corner case fixes Sakari Ailus 2019-01-08 8:58 ` [PATCH v2 1/3] videobuf2-core: Prevent size alignment wrapping buffer size to 0 Sakari Ailus 2019-01-08 12:52 ` Mauro Carvalho Chehab 2019-01-08 12:59 ` Mauro Carvalho Chehab 2019-01-08 13:01 ` Mauro Carvalho Chehab 2019-01-08 13:38 ` Sakari Ailus 2019-01-08 14:23 ` Mauro Carvalho Chehab 2019-01-09 8:41 ` Sakari Ailus 2019-01-08 13:40 ` Sakari Ailus 2019-01-08 14:30 ` Mauro Carvalho Chehab 2019-01-08 16:05 ` Laurent Pinchart 2019-01-09 12:13 ` Mauro Carvalho Chehab 2019-01-09 13:56 ` Sakari Ailus 2019-01-08 8:58 ` [PATCH v2 2/3] videobuf2-dma-sg: Prevent size from overflowing Sakari Ailus 2019-01-08 13:09 ` Mauro Carvalho Chehab 2019-01-08 13:29 ` Sakari Ailus 2019-01-08 13:44 ` Mauro Carvalho Chehab 2019-01-08 13:57 ` Sakari Ailus 2019-01-08 8:58 ` [PATCH v2 3/3] videobuf2-core.h: Document the alloc memop size argument as page aligned Sakari Ailus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).