Hi Hans,
On Sat, Nov 6, 2021 at 9:39 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Hi,
>
> On 11/1/21 15:53, Hans de Goede wrote:
> > Commit a4b83deb3e76 ("media: videobuf2: rework vb2_mem_ops API")
> > added a new vb member to struct vb2_dma_sg_buf, but it only added
> > code setting this to the vb2_dma_sg_alloc() function and not to the
> > vb2_dma_sg_get_userptr() and vb2_dma_sg_attach_dmabuf() which also
> > create vb2_dma_sg_buf objects.
> >
> > This is causing a crash due to a NULL pointer deref when using
> > libcamera on devices with an Intel IPU3 (qcam app).
> >
> > Fix these crashes by assigning buf->vb in the other 2 functions too,
> > note libcamera tests the vb2_dma_sg_get_userptr() path, the change
> > to the vb2_dma_sg_attach_dmabuf() path is untested.
> >
> > Fixes: a4b83deb3e76 ("media: videobuf2: rework vb2_mem_ops API")
> > Cc: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>
> Ping ? This is still an issue in the current media-staging tree.
Uh, sorry, I thought this was already fixed by [1], but that was only
for the dma-contig allocator. Thanks for the patch.
Acked-by: Tomasz Figa <tfiga@xxxxxxxxxxxx>
[1] https://patchwork.kernel.org/project/linux-media/patch/20210928034634.333785-1-senozhatsky@xxxxxxxxxxxx/
Hans (V.), would you pick this fix, please?
Best regards,
Tomasz
>
> Regards,
>
> Hans
>
>
> > ---
> > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > index 33ee63a99139..0452ed9fac95 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > @@ -241,6 +241,7 @@ static void *vb2_dma_sg_get_userptr(struct vb2_buffer *vb, struct device *dev,
> > buf->offset = vaddr & ~PAGE_MASK;
> > buf->size = size;
> > buf->dma_sgt = &buf->sg_table;
> > + buf->vb = vb;
> > vec = vb2_create_framevec(vaddr, size);
> > if (IS_ERR(vec))
> > goto userptr_fail_pfnvec;
> > @@ -642,6 +643,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
> > buf->dma_dir = vb->vb2_queue->dma_dir;
> > buf->size = size;
> > buf->db_attach = dba;
> > + buf->vb = vb;
> >
> > return buf;
> > }
> >
>
