On Tue, Oct 13, 2026 at 5:02 AM Matt Evans <[email protected]> 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]> > --- > 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; > > -- > 2.47.3 >
Reviewed-by: Zack Rusin <[email protected]> z
smime.p7s
Description: S/MIME Cryptographic Signature
