Hi all,
A fix for DMABUF's file-release path, to set dmabuf->file = NULL when the struct file itself is being freed and avoid a stale pointer lingering. Previously it could be assumed that dmabuf->file was always valid until .release was called, but commit 4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to dentry_ops") separated dentry release (calls .release) from file release (can happen before .release). Setting the pointer to NULL clearly delimits the period in which it points to a valid struct file.
Raw dmabuf->file access is bad news unless the file is known to have a reference held (for example, during export). A single-threaded problem scenario is a dentry held open (e.g. using O_PATH) but the file closed; .release is not called, so the exporter was unaware that dmabuf->file became stale. Exporter activities using dmabuf->file then led to a UAF. Or, threads racing concurrent access with a file closing.
VFIO uses get_file_active(&dmabuf->file), which is a UAF when the pointer goes stale. Alex Mastro has created a VFIO-centric reproducer [1], which shows up with KASAN.
To fix, dmabuf->file is either valid or NULL. get_file_active() is then safe and a recommended way to access dmabuf->file without an existing reference. The only possible outcomes are for it to get a reference to the file, to observe a valid pointer but the file in the DEAD state (being closed, fail), or NULL (fail).
Patch [3/3] sets the pointer NULL when the file closes, which get_file_active() handles correctly.
vmwgfx was identified as doing a problematic dmabuf->file dereference. Although already a UAF, making dmabuf->file = NULL turns it into an oops. So, patch [1/3] correspondingly converts vmwgfx to use get_file_active() too.
i915 GVT unsafely dereferences dmabuf->file for a trivial debug printout after the file reference moves to the dmabuf's fd, so patch [2/3] dereferences in the safe window.
Added comments to highlight that dmabuf->file could go away before .release, and to use get_file_active() if a reference isn't otherwise already held.
Thanks,
Matt
[1]: https://github.com/opsound/vfio-dmabuf-lab/blob/main/tests/vfio_dmabuf_opath...
Changes:
v3: - Better comment for struct dma_buf's file (refer to synchronisation against ->release). - Added trivial i915 patch to remove unsafe debug dmabuf->file accesss.
v2: https://lore.kernel.org/all/20261009123601.94079-1-matt@ozlabs.org/
"v1" was the single "dma-buf: Annul dmabuf->file on file release" patch:
https://lore.kernel.org/all/f8efaabd-c06e-4f04-8cfa-489c148e37ba@ozlabs.org/
Matt Evans (3): drm/vmwgfx: Use get_file_active() for dmabuf->file drm/i915/gvt: Read dmabuf->file only while valid dma-buf: Annul dmabuf->file on file release
drivers/dma-buf/dma-buf.c | 21 ++++++++++++++++++--- drivers/gpu/drm/i915/gvt/dmabuf.c | 5 +++-- drivers/gpu/drm/vmwgfx/ttm_object.c | 2 +- include/linux/dma-buf.h | 5 ++++- 4 files changed, 26 insertions(+), 7 deletions(-)