Commit 33927f3d authored by Al Viro's avatar Al Viro
Browse files

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: db1a8dd9 ("habanalabs: add support for dma-buf exporter")
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent d7b8f8e2

drivers/accel/habanalabs/common/memory.c

+7 −16
Original line number Diff line number Diff line
@@ -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)