Skip to content

Commit f49856f

Browse files
thejhvivekkreddy
authored andcommitted
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 5e72b2b ("udmabuf: convert udmabuf driver to use folios"); but the memory leak seems to have existed since the introduction of udmabuf. Fixes: fbb0de7 ("Add udmabuf misc device") Acked-by: Vivek Kasireddy <[email protected]> Signed-off-by: Jann Horn <[email protected]> Signed-off-by: Vivek Kasireddy <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 0a16e24 commit f49856f

File tree

1 file changed

+17
-11
lines changed

1 file changed

+17
-11
lines changed

drivers/dma-buf/udmabuf.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -317,24 +317,18 @@ static int check_memfd_seals(struct file *memfd)
317317
return 0;
318318
}
319319

320-
static int export_udmabuf(struct udmabuf *ubuf,
321-
struct miscdevice *device,
322-
u32 flags)
320+
static struct dma_buf *export_udmabuf(struct udmabuf *ubuf,
321+
struct miscdevice *device)
323322
{
324323
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
325-
struct dma_buf *buf;
326324

327325
ubuf->device = device;
328326
exp_info.ops = &udmabuf_ops;
329327
exp_info.size = ubuf->pagecount << PAGE_SHIFT;
330328
exp_info.priv = ubuf;
331329
exp_info.flags = O_RDWR;
332330

333-
buf = dma_buf_export(&exp_info);
334-
if (IS_ERR(buf))
335-
return PTR_ERR(buf);
336-
337-
return dma_buf_fd(buf, flags);
331+
return dma_buf_export(&exp_info);
338332
}
339333

340334
static long udmabuf_pin_folios(struct udmabuf *ubuf, struct file *memfd,
@@ -391,6 +385,7 @@ static long udmabuf_create(struct miscdevice *device,
391385
struct folio **folios = NULL;
392386
pgoff_t pgcnt = 0, pglimit;
393387
struct udmabuf *ubuf;
388+
struct dma_buf *dmabuf;
394389
long ret = -EINVAL;
395390
u32 i, flags;
396391

@@ -455,9 +450,20 @@ static long udmabuf_create(struct miscdevice *device,
455450
}
456451

457452
flags = head->flags & UDMABUF_FLAGS_CLOEXEC ? O_CLOEXEC : 0;
458-
ret = export_udmabuf(ubuf, device, flags);
459-
if (ret < 0)
453+
dmabuf = export_udmabuf(ubuf, device);
454+
if (IS_ERR(dmabuf)) {
455+
ret = PTR_ERR(dmabuf);
460456
goto err;
457+
}
458+
/*
459+
* Ownership of ubuf is held by the dmabuf from here.
460+
* If the following dma_buf_fd() fails, dma_buf_put() cleans up both the
461+
* dmabuf and the ubuf (through udmabuf_ops.release).
462+
*/
463+
464+
ret = dma_buf_fd(dmabuf, flags);
465+
if (ret < 0)
466+
dma_buf_put(dmabuf);
461467

462468
kvfree(folios);
463469
return ret;

0 commit comments

Comments
 (0)