Re: [PATCH] dma-buf: add get_dma_buf()

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, May 22, 2012 at 03:47:12PM +0200, Tomasz Stanislawski wrote:
> Hi,
> I think I discovered an interesting issue with dma_buf.
> I found out that dma_buf_fd does not increase reference
> count for dma_buf::file. This leads to potential kernel
> crash triggered by user space. Please, take a look on
> the scenario below:
> 
> The applications spawns two thread. One of them is exporting DMABUF.
> 
>       Thread I         |   Thread II       | Comments
> -----------------------+-------------------+-----------------------------------
> dbuf = dma_buf_export  |                   | dma_buf is creates, refcount is 1
> fd = dma_buf_fd(dbuf)  |                   | assume fd is set to 42, refcount is still 1
>                        |      close(42)    | The file descriptor is closed asynchronously, dbuf's refcount drops to 0
>                        |  dma_buf_release  | dbuf structure is freed, dbuf becomes a dangling pointer
> int size = dbuf->size; |                   | the dbuf is dereferenced, causing a kernel crash
> -----------------------+-------------------+-----------------------------------
> 
> I think that the problem could be fixed in two ways.
> a) forcing driver developer to call get_dma_buf just before calling dma_buf_fd.
> b) increasing dma_buf->file's reference count at dma_buf_fd
> 
> I prefer solution (b) because it prevents symmetry between dma_buf_fd and close.
> I mean that dma_buf_fd increases reference count, close decreases it.
> 
> What is your opinion about the issue?

I guess most exporters would like to hang onto the exported dma_buf a bit
and hence need a reference (e.g. to cache the dma_buf as long as the
underlying buffer object exists). So I guess we can change the semantics
of dma_buf_fd from transferring the reference you currently have (and
hence forbidding any further access by the caller) to grabbing a reference
of it's on for the fd that is created.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48