Re: [PATCH v3 3/3] dma-buf: Annul dmabuf->file on file release

[Matt Evans <matt@ozlabs.org>]

Hello DMA and DRM folks,

On 13/10/2026 13:13, Christian König wrote:
> 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: 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>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>

Thank you.  Zack's also added R-Bs for 1/3 and 3/3, and Zhenyu an Ack on 
2/3.

If this is acceptable, which tree would you recommend could take this fix?

Thanks,

Matt

> 
>> ---
>>   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;
>>   
>