Commit f49856f5 authored by Jann Horn's avatar Jann Horn Committed by Vivek Kasireddy
Browse files

udmabuf: fix memory leak on last export_udmabuf() error path 



In export_udmabuf(), if dma_buf_fd() fails because the FD table is full, a
dma_buf owning the udmabuf has already been created; but the error handling
in udmabuf_create() will tear down the udmabuf without doing anything about
the containing dma_buf.

This leaves a dma_buf in memory that contains a dangling pointer; though
that doesn't seem to lead to anything bad except a memory leak.

Fix it by moving the dma_buf_fd() call out of export_udmabuf() so that we
can give it different error handling.

Note that the shape of this code changed a lot in commit 5e72b2b4
("udmabuf: convert udmabuf driver to use folios"); but the memory leak
seems to have existed since the introduction of udmabuf.

Fixes: fbb0de79 ("Add udmabuf misc device")
Acked-by: default avatarVivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: default avatarJann Horn <jannh@google.com>
Signed-off-by: default avatarVivek Kasireddy <vivek.kasireddy@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241204-udmabuf-fixes-v2-3-23887289de1c@google.com
parent 0a16e24e

drivers/dma-buf/udmabuf.c

+17 −11
Original line number Diff line number Diff line
@@ -317,12 +317,10 @@ static int check_memfd_seals(struct file *memfd) 
	return 0;

}



static int export_udmabuf(struct udmabuf *ubuf,

			  struct miscdevice *device,

			  u32 flags)

static struct dma_buf *export_udmabuf(struct udmabuf *ubuf,

				      struct miscdevice *device)

{

	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);

	struct dma_buf *buf;



	ubuf->device = device;

	exp_info.ops  = &udmabuf_ops;
@@ -330,11 +328,7 @@ static int export_udmabuf(struct udmabuf *ubuf, 
	exp_info.priv = ubuf;

	exp_info.flags = O_RDWR;



	buf = dma_buf_export(&exp_info);

	if (IS_ERR(buf))

		return PTR_ERR(buf);



	return dma_buf_fd(buf, flags);

	return dma_buf_export(&exp_info);

}



static long udmabuf_pin_folios(struct udmabuf *ubuf, struct file *memfd,
@@ -391,6 +385,7 @@ static long udmabuf_create(struct miscdevice *device, 
	struct folio **folios = NULL;

	pgoff_t pgcnt = 0, pglimit;

	struct udmabuf *ubuf;

	struct dma_buf *dmabuf;

	long ret = -EINVAL;

	u32 i, flags;
@@ -455,9 +450,20 @@ static long udmabuf_create(struct miscdevice *device, 
	}



	flags = head->flags & UDMABUF_FLAGS_CLOEXEC ? O_CLOEXEC : 0;

	ret = export_udmabuf(ubuf, device, flags);

	if (ret < 0)

	dmabuf = export_udmabuf(ubuf, device);

	if (IS_ERR(dmabuf)) {

		ret = PTR_ERR(dmabuf);

		goto err;

	}

	/*

	 * Ownership of ubuf is held by the dmabuf from here.

	 * If the following dma_buf_fd() fails, dma_buf_put() cleans up both the

	 * dmabuf and the ubuf (through udmabuf_ops.release).

	 */



	ret = dma_buf_fd(dmabuf, flags);

	if (ret < 0)

		dma_buf_put(dmabuf);



	kvfree(folios);

	return ret;