[PATCH 1/2] habanalabs: fix UAF in export_dmabuf()

[Lukas Wunner]

On Sat, Jul 12, 2025 at 06:02:31AM +0100, Al Viro wrote:
> [dma_buf_fd() fixes; no preferences regarding the tree it goes through -
> up to dri folks]

A MAINTAINERS change for the habanalabs driver landed in Linus' tree
a few hours ago.  I'm adding the new maintainers Yaron, Koby and
Konstantin to the To: header.  Thanks! -- Lukas

> As soon as we'd inserted a file reference into descriptor table, another
> thread could close it.  That's fine for the case when all we are doing is
> returning that descriptor to userland (it's a race, but it's a userland
> race and there's nothing the kernel can do about it).  However, if we
> follow fd_install() with any kind of access to objects that would be
> destroyed on close (be it the struct file itself or anything destroyed
> by its ->release()), we have a UAF.
> 
> dma_buf_fd() is a combination of reserving a descriptor and fd_install().
> habanalabs export_dmabuf() calls it and then proceeds to access the
> objects destroyed on close.  In particular, it grabs an extra reference to
> another struct file that will be dropped as part of ->release() for ours;
> that "will be" is actually "might have already been".
> 
> Fix that by reserving descriptor before anything else and do fd_install()
> only when everything had been set up.  As a side benefit, we no longer
> have the failure exit with file already created, but reference to
> underlying file (as well as ->dmabuf_export_cnt, etc.) not grabbed yet;
> unlike dma_buf_fd(), fd_install() can't fail.
> 
> Fixes: db1a8dd916aa ("habanalabs: add support for dma-buf exporter")
> Signed-off-by: Al Viro <viro at zeniv.linux.org.uk>
> ---
>  drivers/accel/habanalabs/common/memory.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/accel/habanalabs/common/memory.c b/drivers/accel/habanalabs/common/memory.c
> index 601fdbe70179..61472a381904 100644
> --- a/drivers/accel/habanalabs/common/memory.c
> +++ b/drivers/accel/habanalabs/common/memory.c
> @@ -1829,9 +1829,6 @@ static void hl_release_dmabuf(struct dma_buf *dmabuf)
>  	struct hl_dmabuf_priv *hl_dmabuf = dmabuf->priv;
>  	struct hl_ctx *ctx;
>  
> -	if (!hl_dmabuf)
> -		return;
> -
>  	ctx = hl_dmabuf->ctx;
>  
>  	if (hl_dmabuf->memhash_hnode)
> @@ -1859,7 +1856,12 @@ static int export_dmabuf(struct hl_ctx *ctx,
>  {
>  	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>  	struct hl_device *hdev = ctx->hdev;
> -	int rc, fd;
> +	CLASS(get_unused_fd, fd)(flags);
> +
> +	if (fd < 0) {
> +		dev_err(hdev->dev, "failed to get a file descriptor for a dma-buf, %d\n", fd);
> +		return fd;
> +	}
>  
>  	exp_info.ops = &habanalabs_dmabuf_ops;
>  	exp_info.size = total_size;
> @@ -1872,13 +1874,6 @@ static int export_dmabuf(struct hl_ctx *ctx,
>  		return PTR_ERR(hl_dmabuf->dmabuf);
>  	}
>  
> -	fd = dma_buf_fd(hl_dmabuf->dmabuf, flags);
> -	if (fd < 0) {
> -		dev_err(hdev->dev, "failed to get a file descriptor for a dma-buf, %d\n", fd);
> -		rc = fd;
> -		goto err_dma_buf_put;
> -	}
> -
>  	hl_dmabuf->ctx = ctx;
>  	hl_ctx_get(hl_dmabuf->ctx);
>  	atomic_inc(&ctx->hdev->dmabuf_export_cnt);
> @@ -1890,13 +1885,9 @@ static int export_dmabuf(struct hl_ctx *ctx,
>  	get_file(ctx->hpriv->file_priv->filp);
>  
>  	*dmabuf_fd = fd;
> +	fd_install(take_fd(fd), hl_dmabuf->dmabuf->file);
>  
>  	return 0;
> -
> -err_dma_buf_put:
> -	hl_dmabuf->dmabuf->priv = NULL;
> -	dma_buf_put(hl_dmabuf->dmabuf);
> -	return rc;
>  }
>  
>  static int validate_export_params_common(struct hl_device *hdev, u64 addr, u64 size, u64 offset)
> -- 
> 2.39.5