On 10/9/26 14:35, 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: stable@vger.kernel.org Fixes: 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to dentry_ops") Signed-off-by: Matt Evans matt@ozlabs.org

---

drivers/dma-buf/dma-buf.c | 21 ++++++++++++++++++--- include/linux/dma-buf.h | 4 +++- 2 files changed, 21 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..9f3c92c71c29 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -303,7 +303,9 @@ 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().  Exporters can use
  

• * get_file_active(&dma_buf->file) to safely acquire a reference;
  

Please add something like "* when the exporter holds a common lock with the ->release() callback".

Apart from that looks good to me.

Regards, Christian.

• * the file could otherwise be closed at any time.
  

  */ struct file *file;