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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > Reviewed-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxx
