[PATCH v2 0/2] dma-buf: dmabuf->file release-time cleanup

[PATCH v2 0/2] dma-buf: dmabuf->file release-time cleanup

[Matt Evans]

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 [2/2] 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, the paired patch [1/2] correspondingly converts vmwgfx to
use get_file_active() too.

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_uaf_test.c

"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 (2):
  drm/vmwgfx: Use get_file_active() for dmabuf->file
  dma-buf: Annul dmabuf->file on file release

 drivers/dma-buf/dma-buf.c           | 21 ++++++++++++++++++---
 drivers/gpu/drm/vmwgfx/ttm_object.c |  2 +-
 include/linux/dma-buf.h             |  4 +++-
 3 files changed, 22 insertions(+), 5 deletions(-)

-- 
2.47.3

[PATCH v2 1/2] drm/vmwgfx: Use get_file_active() for dmabuf->file

[Matt Evans]

4ab59c3c638c6 ("dma-buf: Move dma_buf_release() from fops to
dentry_ops") moved the dmabuf .release, and broke the vmwgfx driver's
assumption that dmabuf->file would remain valid until release.
Currently, dmabuf->file can be stale after the file closes even if
.release has not yet been called, and a future commit makes
dmabuf->file become NULL instead of stale.

In preparation for that future commit, convert
get_dma_buf_unless_doomed() to use get_file_active(&dmabuf->file)
which is the safe method to access and get a reference, even when
dmabuf->file could be NULL.

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/gpu/drm/vmwgfx/ttm_object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/ttm_object.c b/drivers/gpu/drm/vmwgfx/ttm_object.c
index f9042bafdc93..9137a31eaa1a 100644
--- a/drivers/gpu/drm/vmwgfx/ttm_object.c
+++ b/drivers/gpu/drm/vmwgfx/ttm_object.c
@@ -471,7 +471,7 @@ void ttm_object_device_release(struct ttm_object_device **p_tdev)
  */
 static bool __must_check get_dma_buf_unless_doomed(struct dma_buf *dmabuf)
 {
-	return file_ref_get(&dmabuf->file->f_ref);
+	return get_file_active(&dmabuf->file) != NULL;
 }
 
 /**
-- 
2.47.3

[PATCH v2 2/2] dma-buf: Annul dmabuf->file on file release

[Matt Evans]

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;
+	 * the file could otherwise be closed at any time.
 	 */
 	struct file *file;
 
-- 
2.47.3

Re: [PATCH v2 2/2] dma-buf: Annul dmabuf->file on file release

[Christian König]

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

Re: [PATCH v2 2/2] dma-buf: Annul dmabuf->file on file release

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dma-buf: NULL pointer dereference of dmabuf->file in exporter drivers
--

commit b5be810d5fa3244638a9bad8503cacb8ca86027e
Author: Matt Evans <matt@ozlabs.org>

dma-buf: Annul dmabuf->file on file release

This patch sets dmabuf->file to NULL in dma_buf_file_release() to prevent a
use-after-free window if the dentry outlives the file. By clearing the
pointer, exporters using get_file_active() can safely observe that the file
is closed.

> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 36cdc342627ee..d0f8b766e8fd1 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);

[Severity: High]
Does setting dmabuf->file to NULL here cause a NULL pointer dereference in
exporter drivers that still hold a reference to the dma_buf?

For example, if a GEM object is exported to a DMA-BUF and the file descriptor
is closed, the dma_buf is kept alive by the GEM object. If a user then reads
the debugfs file, amdgpu_bo_print_info() in
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c dereferences dmabuf->file:

		seq_printf(m, " imported from ino:%llu", file_inode(dma_buf->file)->i_ino);
	else if (dma_buf)
		seq_printf(m, " exported as ino:%llu", file_inode(dma_buf->file)->i_ino);

Similarly, during cleanup, amdgpu_ualink_cleanup_imp_xa_node() in
drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c attempts to read the file count:

	dev_dbg(adev->dev,
		"IMP-CLEANUP: dmabuf free for NPA:%llx handle:%llx:%llx, fc:%lu\n",
		imp_xa_node->npa_addr, imp_xa_node->handle.handle_hi,
		imp_xa_node->handle.handle_lo,
		file_count(imp_xa_node->dmabuf->file));

Could these routines trigger a kernel panic when dmabuf->file is NULL?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009123601.94079-1-matt@ozlabs.org?part=2

Re: [PATCH v2 2/2] dma-buf: Annul dmabuf->file on file release

[Matt Evans]

Hi Christian, Alex,

I'd be very grateful for your help on this:

On 09/10/2026 14:11, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] dma-buf: NULL pointer dereference of dmabuf->file in exporter drivers
> --
> 
> commit b5be810d5fa3244638a9bad8503cacb8ca86027e
> Author: Matt Evans <matt@ozlabs.org>
> 
> dma-buf: Annul dmabuf->file on file release
> 
> This patch sets dmabuf->file to NULL in dma_buf_file_release() to prevent a
> use-after-free window if the dentry outlives the file. By clearing the
> pointer, exporters using get_file_active() can safely observe that the file
> is closed.
> 
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 36cdc342627ee..d0f8b766e8fd1 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);
> 
> [Severity: High]
> Does setting dmabuf->file to NULL here cause a NULL pointer dereference in
> exporter drivers that still hold a reference to the dma_buf?
> 
> For example, if a GEM object is exported to a DMA-BUF and the file descriptor
> is closed, the dma_buf is kept alive by the GEM object. If a user then reads
> the debugfs file, amdgpu_bo_print_info() in
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c dereferences dmabuf->file:
> 
> 		seq_printf(m, " imported from ino:%llu", file_inode(dma_buf->file)->i_ino);
> 	else if (dma_buf)
> 		seq_printf(m, " exported as ino:%llu", file_inode(dma_buf->file)->i_ino);
> 
> Similarly, during cleanup, amdgpu_ualink_cleanup_imp_xa_node() in
> drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c attempts to read the file count:
> 
> 	dev_dbg(adev->dev,
> 		"IMP-CLEANUP: dmabuf free for NPA:%llx handle:%llx:%llx, fc:%lu\n",
> 		imp_xa_node->npa_addr, imp_xa_node->handle.handle_hi,
> 		imp_xa_node->handle.handle_lo,
> 		file_count(imp_xa_node->dmabuf->file));
> 
> Could these routines trigger a kernel panic when dmabuf->file is NULL?

I am out on a limb here, being the first time I've read this driver. :-I 
  It _looks_ like there's a live dmabuf->file because a 
get_dma_buf()-style reference is held here (so a false positive from 
Sashiko).  But I'm quite uncertain.

Christian, that you hadn't already mentioned this hints to me that this 
is safe, is that fair?


Thanks,


Matt

Re: [PATCH v2 2/2] dma-buf: Annul dmabuf->file on file release

[Christian König]

On 10/12/26 17:27, Matt Evans wrote:
> Hi Christian, Alex,
> 
> I'd be very grateful for your help on this:
> 
> On 09/10/2026 14:11, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] dma-buf: NULL pointer dereference of dmabuf->file in exporter drivers
>> -- 
>>
>> commit b5be810d5fa3244638a9bad8503cacb8ca86027e
>> Author: Matt Evans <matt@ozlabs.org>
>>
>> dma-buf: Annul dmabuf->file on file release
>>
>> This patch sets dmabuf->file to NULL in dma_buf_file_release() to prevent a
>> use-after-free window if the dentry outlives the file. By clearing the
>> pointer, exporters using get_file_active() can safely observe that the file
>> is closed.
>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index 36cdc342627ee..d0f8b766e8fd1 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);
>>
>> [Severity: High]
>> Does setting dmabuf->file to NULL here cause a NULL pointer dereference in
>> exporter drivers that still hold a reference to the dma_buf?
>>
>> For example, if a GEM object is exported to a DMA-BUF and the file descriptor
>> is closed, the dma_buf is kept alive by the GEM object. If a user then reads
>> the debugfs file, amdgpu_bo_print_info() in
>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c dereferences dmabuf->file:
>>
>>         seq_printf(m, " imported from ino:%llu", file_inode(dma_buf->file)->i_ino);
>>     else if (dma_buf)
>>         seq_printf(m, " exported as ino:%llu", file_inode(dma_buf->file)->i_ino);
>>
>> Similarly, during cleanup, amdgpu_ualink_cleanup_imp_xa_node() in
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c attempts to read the file count:
>>
>>     dev_dbg(adev->dev,
>>         "IMP-CLEANUP: dmabuf free for NPA:%llx handle:%llx:%llx, fc:%lu\n",
>>         imp_xa_node->npa_addr, imp_xa_node->handle.handle_hi,
>>         imp_xa_node->handle.handle_lo,
>>         file_count(imp_xa_node->dmabuf->file));
>>
>> Could these routines trigger a kernel panic when dmabuf->file is NULL?
> 
> I am out on a limb here, being the first time I've read this driver. :-I  It _looks_ like there's a live dmabuf->file because a get_dma_buf()-style reference is held here (so a false positive from Sashiko).  But I'm quite uncertain.
> 
> Christian, that you hadn't already mentioned this hints to me that this is safe, is that fair?

The Sashiko comment is nonsense. Each GEM object which has a DMA-buf (either through import or export) holds a reference to the DMA-buf file.

The only case where it is temporarily not holding the reference is during the destruction dance, e.g. handle ref goes to zero, DMA-buf is released and freed, GEM object is released and freed.

Additional to that this is for debugfs which is meant to be used for debugging only and is not really stable. We literally have tons of debugfs files which do very questionable things with locks and/or RCU and sometimes just returns random data because the object in question is about to be destroyed.

A NULL pointer deref is a bit more problematic. But in general most people don't care much, it's debugfs.

Regards,
Christian.

> 
> 
> Thanks,
> 
> 
> Matt

Re: [PATCH v2 2/2] dma-buf: Annul dmabuf->file on file release

[Matt Evans]

Hi Christian,

On 12/10/2026 18:48, Christian König wrote:
> On 10/12/26 17:27, Matt Evans wrote:
>> Hi Christian, Alex,
>>
>> I'd be very grateful for your help on this:
>>
>> On 09/10/2026 14:11, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>> - [High] dma-buf: NULL pointer dereference of dmabuf->file in exporter drivers
>>> -- 
>>>
>>> commit b5be810d5fa3244638a9bad8503cacb8ca86027e
>>> Author: Matt Evans <matt@ozlabs.org>
>>>
>>> dma-buf: Annul dmabuf->file on file release
>>>
>>> This patch sets dmabuf->file to NULL in dma_buf_file_release() to prevent a
>>> use-after-free window if the dentry outlives the file. By clearing the
>>> pointer, exporters using get_file_active() can safely observe that the file
>>> is closed.
>>>
>>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>>> index 36cdc342627ee..d0f8b766e8fd1 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);
>>>
>>> [Severity: High]
>>> Does setting dmabuf->file to NULL here cause a NULL pointer dereference in
>>> exporter drivers that still hold a reference to the dma_buf?
>>>
>>> For example, if a GEM object is exported to a DMA-BUF and the file descriptor
>>> is closed, the dma_buf is kept alive by the GEM object. If a user then reads
>>> the debugfs file, amdgpu_bo_print_info() in
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c dereferences dmabuf->file:
>>>
>>>          seq_printf(m, " imported from ino:%llu", file_inode(dma_buf->file)->i_ino);
>>>      else if (dma_buf)
>>>          seq_printf(m, " exported as ino:%llu", file_inode(dma_buf->file)->i_ino);
>>>
>>> Similarly, during cleanup, amdgpu_ualink_cleanup_imp_xa_node() in
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c attempts to read the file count:
>>>
>>>      dev_dbg(adev->dev,
>>>          "IMP-CLEANUP: dmabuf free for NPA:%llx handle:%llx:%llx, fc:%lu\n",
>>>          imp_xa_node->npa_addr, imp_xa_node->handle.handle_hi,
>>>          imp_xa_node->handle.handle_lo,
>>>          file_count(imp_xa_node->dmabuf->file));
>>>
>>> Could these routines trigger a kernel panic when dmabuf->file is NULL?
>>
>> I am out on a limb here, being the first time I've read this driver. :-I  It _looks_ like there's a live dmabuf->file because a get_dma_buf()-style reference is held here (so a false positive from Sashiko).  But I'm quite uncertain.
>>
>> Christian, that you hadn't already mentioned this hints to me that this is safe, is that fair?
> 
> The Sashiko comment is nonsense. Each GEM object which has a DMA-buf (either through import or export) holds a reference to the DMA-buf file.
> 
> The only case where it is temporarily not holding the reference is during the destruction dance, e.g. handle ref goes to zero, DMA-buf is released and freed, GEM object is released and freed.
> 
> Additional to that this is for debugfs which is meant to be used for debugging only and is not really stable. We literally have tons of debugfs files which do very questionable things with locks and/or RCU and sometimes just returns random data because the object in question is about to be destroyed.
> 
> A NULL pointer deref is a bit more problematic. But in general most people don't care much, it's debugfs.

That makes sense, and I really appreciate the explanation.  Thanks for 
the help.


Matt