On 10/13/26 14:02, Matt Evans wrote:
> The dmabuf release path is split between file and dentry release:
> dma_buf_file_release() is called shortly before the file is freed, and
> dma_buf_release() calls an exporter's dmabuf->ops->release when the
> dentry is freed. However, the dentry can outlive the file (for
> example, if opened with O_PATH), meaning .release might be called some
> time after the file is freed.
>
> This presents a window, when closing a DMABUF file, in which
> dmabuf->file points to freed memory yet .release op has not yet been
> called.
>
> For VFIO, if a buffer's .release has not been called it's considered
> still active and subject to move/cleanup. If so, it attempts to
> get_file_active() on dmabuf->file: a file close with dentry held open
> will call this function with a stale pointer, a UAF.
>
> To make this pattern safe, set dmabuf->file to NULL in
> dma_buf_file_release() to reflect that the associated file is now dead
> even if the DMABUF is not yet gone. A get_file_active() ... fput()
> sequence concurrent with a file close then only executes as one of:
>
> - Gets the file before file_ref_put() (dmabuf->file valid)
> - Observes dmabuf->file pointing to a file, but it's DEAD (no file)
> - Observes dmabuf->file = NULL (no file)
>
> Originally, drivers could assume dmabuf->file was valid until .release
> was called from fops->release. This assumption was no longer valid
> after 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to
> dentry_ops"), which moved the callback to the dentry release (by which
> point the file might have been freed). With this commit, drivers must
> still consider that dmabuf->file could be NULL before .release.
>
> Cc: [email protected]
> Fixes: 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to
> dentry_ops")
> Signed-off-by: Matt Evans <[email protected]>
Reviewed-by: Christian König <[email protected]>
> ---
> drivers/dma-buf/dma-buf.c | 21 ++++++++++++++++++---
> include/linux/dma-buf.h | 5 ++++-
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 4c9add51f9ef..61485fc28b8b 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -193,11 +193,26 @@ static void dma_buf_release(struct dentry *dentry)
>
> static int dma_buf_file_release(struct inode *inode, struct file *file)
> {
> - if (!is_dma_buf_file(file))
> - return -EINVAL;
> + struct dma_buf *dmabuf = file->private_data;
> +
> + if (!dmabuf)
> + return 0;
>
> - __dma_buf_list_del(file->private_data);
> + __dma_buf_list_del(dmabuf);
>
> + /*
> + * As the file is being closed, dmabuf->file is no longer
> + * valid and NULL replaces an otherwise stale pointer. If a
> + * reference to the file isn't already held, the file can be
> + * closed at any time, including before dmabuf->ops->release()
> + * happens: dmabuf->file _must not_ be accessed directly. An
> + * exporter should use get_file_active(&dmabuf->file) which
> + * atomically tests and acquires a reference to the file (or
> + * returns NULL if the file has closed). The barrier ensures
> + * get_file_active() observes the store before file_free()
> + * frees the memory.
> + */
> + smp_store_mb(dmabuf->file, NULL);
> return 0;
> }
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index d15b2b31d3c9..a31eb0ab9ecf 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -303,7 +303,10 @@ struct dma_buf {
> * @file:
> *
> * File pointer used for sharing buffers across, and for refcounting.
> - * See dma_buf_get() and dma_buf_put().
> + * See dma_buf_get() and dma_buf_put(). When an exporter holds a
> + * common lock with the ->release() callback, it can use
> + * get_file_active(&dma_buf->file) to safely acquire a reference;
> + * the file could otherwise be closed at any time.
> */
> struct file *file;
>
