Hi Alex,
On 07/10/2026 10:19, Alex Mastro wrote:
> On Tue, Oct 06, 2026 at 08:15:26PM +0100, Matt Evans wrote:
>> static int dma_buf_file_release(struct inode *inode, struct file *file)
>> {
>> + struct dma_buf *dmabuf = file->private_data;
>
> I think dmabuf can be NULL here if the dmabuf allocation fails during
> dma_buf_export().
>
>> +
>> if (!is_dma_buf_file(file))
>> return -EINVAL;
>> - __dma_buf_list_del(file->private_data);
>> + __dma_buf_list_del(dmabuf);
>> + /* Must be observed by __get_file_rcu() before file_free() */
>> + smp_store_mb(dmabuf->file, NULL);
>
> Resulting in NULL deref here -- guard with null check?
Oof, yes! Thanks for catching this. __dma_buf_list_del() protects
itself against a NULL arg. Done.
Thanks,
Matt
>
>> return 0;
>> }
>
> via this path
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index d504c636dc29..557cd7a4c971 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -725,6 +725,7 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> if (!try_module_get(exp_info->owner))
> return ERR_PTR(-ENOENT);
>
> + // succeeds
> file = dma_buf_getfile(exp_info->size, exp_info->flags);
> if (IS_ERR(file)) {
> ret = PTR_ERR(file);
> @@ -739,6 +740,7 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> dmabuf = kzalloc(alloc_size, GFP_KERNEL);
> if (!dmabuf) {
> ret = -ENOMEM;
> + // go here
> goto err_file;
> }
>
> @@ -771,6 +773,7 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> return dmabuf;
>
> err_file:
> + // ends up calling dma_buf_file_release()
> fput(file);
> err_module:
> module_put(exp_info->owner);
>
