habanalabs: fix UAF in export_dmabuf()

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@zeniv.linux.org.uk>

diff --git a/drivers/accel/habanalabs/common/memory.c b/drivers/accel/habanalabs/common/memory.c
index 601fdbe701790b2d242949e394990caa6783e30b..61472a381904ec2c8bab559e3fd16528f095edf9 100644 (file)
--- 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)