Hi Laurent,
On 06/19/2012 11:00 PM, Laurent Pinchart wrote:
Hi Tomasz,
Thanks for the patch.
On Thursday 14 June 2012 15:37:40 Tomasz Stanislawski wrote:
This patch removes a reference to alloc_ctx from an instance of a DMA contiguous buffer. It helps to avoid a risk of a dangling pointer if the context is released while the buffer is still valid.
Can this really happen ? All drivers except marvell-ccic seem to call vb2_dma_contig_cleanup_ctx() in their remove handler and probe cleanup path only. Freeing the context while buffers are still around would be a driver bug, and I expect drivers to destroy the queue in that case anyway.
This being said, removing the dereference step is a good idea, so I think the patch should be applied, possibly with a different commit message.
The problem may happen if a DMABUF sharing is used. - process A uses V4L2 queue to create a buffer - process A exports a buffer and shares it with the process B (by sockets or /proc/pid/fd) - the process A gets killed, queue is destroyed - someone call rmmod on v4l driver, alloc_ctx is freed - process B keeps reference to a buffer that has a dangling reference to alloc_ctx
The presented scenario might be a bit too pathological and artificial. Moreover it involves root privileges. But it is possible to trigger this bug. One solution might be keeping reference count in alloc_ctx but it would be easier to get rid of the reference to alloc_ctx from vb2-dma-contig buffer.
BTW. I decided to drop 'Remove unneeded allocation context structure' because Marek Szyprowski is working on extension to vb2-dma-contig that allow to create buffers with no kernel mappings. That feature involved additional parameter to alloc_ctx other than pointer to the device.
Regards, Tomasz Stanislawski
Moreover it removes one dereference step while accessing a device structure.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);