On 15/11/2021 09:54, Tomasz Figa wrote:
> 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?
Yes, it's in a PR I posted for 5.16.
Regards,
Hans
>
> 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;
>>> }
>>>
>>
