[PATCH] dma-buf: dma-heap: close installed fd when copy_to_user() fails

[PATCH] dma-buf: dma-heap: close installed fd when copy_to_user() fails

[Baineng Shou]

DMA_HEAP_IOCTL_ALLOC allocates a dma-buf and installs an fd into the
caller's fd table via fd_install() before dma_heap_ioctl() copies the
result back to userspace. If the trailing copy_to_user() fails, the
ioctl returns -EFAULT and userspace never learns the fd number, but
the fd (and the underlying dma-buf reference) remain in the caller's
fd table and are leaked for the lifetime of the process.

The failure is easily reachable from userspace: pass a struct
dma_heap_allocation_data that lives in a page whose protection is
flipped to PROT_READ between copy_from_user() and copy_to_user()
(e.g. via mprotect()). Each such ioctl leaks one dmabuf fd; repeating
the call quickly fills /proc/<pid>/fd with anonymous "/dmabuf:"
entries that only go away when the process exits.

Fix it by closing the installed fd (and clearing the fd field of the
kernel-side copy) when copy_to_user() fails after a successful
allocation, so the error path matches what userspace observes: no fd
was returned, therefore no fd is left behind.

Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
Cc: stable@vger.kernel.org
Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
---

Reproducer (full source, gcc -o poc poc.c; run as root):

    // poc.c -- leak one dma-buf fd per DMA_HEAP_IOCTL_ALLOC
    //          when copy_to_user() fails
    #include <fcntl.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <unistd.h>
    #include <sys/ioctl.h>
    #include <sys/mman.h>
    #include <linux/dma-heap.h>

    int main(int argc, char **argv)
    {
        int n = argc > 1 ? atoi(argv[1]) : 100;
        long ps = sysconf(_SC_PAGESIZE);

        int heap = open("/dev/dma_heap/system", O_RDWR | O_CLOEXEC);
        if (heap < 0)
            return perror("open"), 1;

        for (int i = 0; i < n; i++) {
            /* Put a valid request in a page, then make the page
             * read-only: copy_from_user() still succeeds and the
             * dma-buf is allocated and fd_install()'d, but the
             * trailing copy_to_user() fails and the fd, never
             * returned to us, is leaked.
             */
            struct dma_heap_allocation_data *req =
                mmap(NULL, ps, PROT_READ | PROT_WRITE,
                     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

            memset(req, 0, sizeof(*req));
            req->len = ps;
            req->fd_flags = O_RDWR | O_CLOEXEC;

            mprotect(req, ps, PROT_READ);
            ioctl(heap, DMA_HEAP_IOCTL_ALLOC, req);  /* -EFAULT */
            munmap(req, ps);
        }

        printf("done: check ls -l /proc/%d/fd for %d leaked fds\n",
               getpid(), n);
        pause();
        return 0;
    }

Before the fix, ./poc 10 leaves 10 anonymous dmabuf fds in the
caller's fd table:

    # ls -l /proc/$(pgrep poc)/fd
    lrwx------ 1 root root 64 Jan  1 00:03 3 -> /dev/dma_heap/system
    lrwx------ 1 root root 64 Jan  1 00:03 4 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 5 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 6 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 7 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 8 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 9 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 10 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 11 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 12 -> /dmabuf:
    lrwx------ 1 root root 64 Jan  1 00:03 13 -> /dmabuf:

After the fix, only /dev/dma_heap/system remains open; the
anonymous "/dmabuf:" entries are gone.

 drivers/dma-buf/dma-heap.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index a76bf3f8b071..0dd7a84b06bf 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -18,6 +18,7 @@
 #include <linux/uaccess.h>
 #include <linux/xarray.h>
 #include <uapi/linux/dma-heap.h>
+#include <linux/fdtable.h>
 
 #define DEVNAME "dma_heap"
 
@@ -181,8 +182,16 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
 		goto err;
 	}
 
-	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)
+	if (copy_to_user((void __user *)arg, kdata, out_size) != 0) {
+		if (kcmd == DMA_HEAP_IOCTL_ALLOC && ret == 0) {
+			struct dma_heap_allocation_data *h = (void *)kdata;
+
+			close_fd(h->fd);
+			h->fd = -1;
+		}
 		ret = -EFAULT;
+	}
+
 err:
 	if (kdata != stack_kdata)
 		kfree(kdata);
-- 
2.34.1

Re: [PATCH] dma-buf: dma-heap: close installed fd when copy_to_user() fails

[Christian König]

On 7/3/26 10:09, Baineng Shou wrote:
> DMA_HEAP_IOCTL_ALLOC allocates a dma-buf and installs an fd into the
> caller's fd table via fd_install() before dma_heap_ioctl() copies the
> result back to userspace. If the trailing copy_to_user() fails, the
> ioctl returns -EFAULT and userspace never learns the fd number, but
> the fd (and the underlying dma-buf reference) remain in the caller's
> fd table and are leaked for the lifetime of the process.

This came up many many times before and is the completely wrong approach to handle that.

Background is that this opens up a race window between install_fd() and close_fd() which is much more worse than the original issue and can be abused for a couple of different things.

If I'm not completely mistaken we now have automated checkers in place which find such use case in the upstream kernel and complain about it.

IIRC the correct approach is to call get_unused_fd_flags() to get an fd, do your things, copy it to user space etc... and then finally do the fd_install(fd, dmabuf->file) when nothing can go wrong any more.

If an error happens use put_unused_fd() to clean up.

Regards,
Christian.

> 
> The failure is easily reachable from userspace: pass a struct
> dma_heap_allocation_data that lives in a page whose protection is
> flipped to PROT_READ between copy_from_user() and copy_to_user()
> (e.g. via mprotect()). Each such ioctl leaks one dmabuf fd; repeating
> the call quickly fills /proc/<pid>/fd with anonymous "/dmabuf:"
> entries that only go away when the process exits.
> 
> Fix it by closing the installed fd (and clearing the fd field of the
> kernel-side copy) when copy_to_user() fails after a successful
> allocation, so the error path matches what userspace observes: no fd
> was returned, therefore no fd is left behind.
> 
> Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
> Cc: stable@vger.kernel.org
> Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
> ---
> 
> Reproducer (full source, gcc -o poc poc.c; run as root):
> 
>     // poc.c -- leak one dma-buf fd per DMA_HEAP_IOCTL_ALLOC
>     //          when copy_to_user() fails
>     #include <fcntl.h>
>     #include <stdio.h>
>     #include <stdlib.h>
>     #include <string.h>
>     #include <unistd.h>
>     #include <sys/ioctl.h>
>     #include <sys/mman.h>
>     #include <linux/dma-heap.h>
> 
>     int main(int argc, char **argv)
>     {
>         int n = argc > 1 ? atoi(argv[1]) : 100;
>         long ps = sysconf(_SC_PAGESIZE);
> 
>         int heap = open("/dev/dma_heap/system", O_RDWR | O_CLOEXEC);
>         if (heap < 0)
>             return perror("open"), 1;
> 
>         for (int i = 0; i < n; i++) {
>             /* Put a valid request in a page, then make the page
>              * read-only: copy_from_user() still succeeds and the
>              * dma-buf is allocated and fd_install()'d, but the
>              * trailing copy_to_user() fails and the fd, never
>              * returned to us, is leaked.
>              */
>             struct dma_heap_allocation_data *req =
>                 mmap(NULL, ps, PROT_READ | PROT_WRITE,
>                      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> 
>             memset(req, 0, sizeof(*req));
>             req->len = ps;
>             req->fd_flags = O_RDWR | O_CLOEXEC;
> 
>             mprotect(req, ps, PROT_READ);
>             ioctl(heap, DMA_HEAP_IOCTL_ALLOC, req);  /* -EFAULT */
>             munmap(req, ps);
>         }
> 
>         printf("done: check ls -l /proc/%d/fd for %d leaked fds\n",
>                getpid(), n);
>         pause();
>         return 0;
>     }
> 
> Before the fix, ./poc 10 leaves 10 anonymous dmabuf fds in the
> caller's fd table:
> 
>     # ls -l /proc/$(pgrep poc)/fd
>     lrwx------ 1 root root 64 Jan  1 00:03 3 -> /dev/dma_heap/system
>     lrwx------ 1 root root 64 Jan  1 00:03 4 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 5 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 6 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 7 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 8 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 9 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 10 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 11 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 12 -> /dmabuf:
>     lrwx------ 1 root root 64 Jan  1 00:03 13 -> /dmabuf:
> 
> After the fix, only /dev/dma_heap/system remains open; the
> anonymous "/dmabuf:" entries are gone.
> 
>  drivers/dma-buf/dma-heap.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index a76bf3f8b071..0dd7a84b06bf 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -18,6 +18,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/xarray.h>
>  #include <uapi/linux/dma-heap.h>
> +#include <linux/fdtable.h>
>  
>  #define DEVNAME "dma_heap"
>  
> @@ -181,8 +182,16 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
>  		goto err;
>  	}
>  
> -	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)
> +	if (copy_to_user((void __user *)arg, kdata, out_size) != 0) {
> +		if (kcmd == DMA_HEAP_IOCTL_ALLOC && ret == 0) {
> +			struct dma_heap_allocation_data *h = (void *)kdata;
> +
> +			close_fd(h->fd);
> +			h->fd = -1;
> +		}
>  		ret = -EFAULT;
> +	}
> +
>  err:
>  	if (kdata != stack_kdata)
>  		kfree(kdata);

[PATCH v2] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds

[Baineng Shou]

DMA_HEAP_IOCTL_ALLOC allocates a dma-buf and installs an fd into the
caller's fd table via dma_buf_fd() -> fd_install() before
dma_heap_ioctl() copies the result back to userspace.  If the trailing
copy_to_user() fails, userspace never learns the fd number, but the
fd (and the underlying dma-buf reference) are already visible to
other threads in the same process and are leaked for the lifetime of
the process.

The obvious "close it on the failure path" fix is unsafe: once
fd_install() has run, another thread can already dup() the fd, send
it via SCM_RIGHTS, or close() it and let its number be reused, so a
subsequent close_fd() from the ioctl path can operate on an unrelated
file.  This was pointed out by Christian König on v1 [1].

Restructure the allocation path so that fd_install() is the last,
unfailable step of a successful ioctl:

  1. heap->ops->allocate()      creates the dma_buf.
  2. get_unused_fd_flags()      reserves an fd number in the caller's
                                fd table without publishing it, so
                                no other thread can observe it.
  3. copy_to_user()             delivers the fd number to userspace;
                                on failure the fd is returned with
                                put_unused_fd() and the dma_buf
                                reference is dropped with
                                dma_buf_put(), leaving no user-
                                visible state behind.
  4. fd_install()               publishes the fd -- from here on the
                                ioctl cannot fail.

To make this possible, dma_heap_ioctl_allocate() is refactored to
return the struct dma_buf * directly (returning ERR_PTR on failure)
so the caller holds the dmabuf reference across steps 3 and 4.
The fd is written into the kdata buffer before copy_to_user() so
the reserved fd number reaches userspace atomically with the install.

The failure at step 3 is easily reachable from userspace: pass a
struct dma_heap_allocation_data that lives in a page whose protection
is flipped to PROT_READ between copy_from_user() and copy_to_user()
(e.g. via mprotect()).  Before this change each such ioctl leaks one
dmabuf fd; after it, the fd table is unchanged on failure and only
/dev/dma_heap/<name> remains open.

No UAPI or heap-driver interface change.

[1] https://lore.kernel.org/dri-devel/175e98de-f414-47d7-81c1-c0fe0a8f7f62@amd.com/

Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
Cc: stable@vger.kernel.org
Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
---
 drivers/dma-buf/dma-heap.c | 80 +++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index a76bf3f8b071..0a9bf62eb06c 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -55,33 +55,6 @@ MODULE_PARM_DESC(mem_accounting,
 		 "Enable cgroup-based memory accounting for dma-buf heap allocations (default=false).");
 EXPORT_SYMBOL_NS_GPL(mem_accounting, "DMA_BUF_HEAP");
 
-static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
-				 u32 fd_flags,
-				 u64 heap_flags)
-{
-	struct dma_buf *dmabuf;
-	int fd;
-
-	/*
-	 * Allocations from all heaps have to begin
-	 * and end on page boundaries.
-	 */
-	len = PAGE_ALIGN(len);
-	if (!len)
-		return -EINVAL;
-
-	dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags);
-	if (IS_ERR(dmabuf))
-		return PTR_ERR(dmabuf);
-
-	fd = dma_buf_fd(dmabuf, fd_flags);
-	if (fd < 0) {
-		dma_buf_put(dmabuf);
-		/* just return, as put will call release and that will free */
-	}
-	return fd;
-}
-
 static int dma_heap_open(struct inode *inode, struct file *file)
 {
 	struct dma_heap *heap;
@@ -99,30 +72,42 @@ static int dma_heap_open(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static long dma_heap_ioctl_allocate(struct file *file, void *data)
+static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data)
 {
 	struct dma_heap_allocation_data *heap_allocation = data;
 	struct dma_heap *heap = file->private_data;
+	struct dma_buf *dmabuf;
 	int fd;
+	size_t len;
 
 	if (heap_allocation->fd)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
+
+	len = PAGE_ALIGN(heap_allocation->len);
+	if (!len)
+		return ERR_PTR(-EINVAL);
+
+	dmabuf = heap->ops->allocate(heap, len, heap_allocation->fd_flags,
+				     heap_allocation->heap_flags);
 
-	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
-				   heap_allocation->fd_flags,
-				   heap_allocation->heap_flags);
-	if (fd < 0)
-		return fd;
+	if (IS_ERR(dmabuf))
+		return dmabuf;
+
+	fd = get_unused_fd_flags(heap_allocation->fd_flags);
+	if (fd < 0) {
+		dma_buf_put(dmabuf);
+		return ERR_PTR(fd);
+	}
 
 	heap_allocation->fd = fd;
 
-	return 0;
+	return dmabuf;
 }
 
 static unsigned int dma_heap_ioctl_cmds[] = {
@@ -138,6 +123,8 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
 	unsigned int in_size, out_size, drv_size, ksize;
 	int nr = _IOC_NR(ucmd);
 	int ret = 0;
+	int fd;
+	struct dma_buf *dmabuf;
 
 	if (nr >= ARRAY_SIZE(dma_heap_ioctl_cmds))
 		return -EINVAL;
@@ -174,15 +161,28 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
 
 	switch (kcmd) {
 	case DMA_HEAP_IOCTL_ALLOC:
-		ret = dma_heap_ioctl_allocate(file, kdata);
+		dmabuf = dma_heap_ioctl_allocate(file, kdata);
+
+		if (IS_ERR(dmabuf)) {
+			ret = PTR_ERR(dmabuf);
+			break;
+		}
+
+		fd = ((struct dma_heap_allocation_data *)kdata)->fd;
+		if (copy_to_user((void __user *)arg, kdata, out_size) != 0) {
+			put_unused_fd(fd);
+			dma_buf_put(dmabuf);
+			ret = -EFAULT;
+		} else {
+			fd_install(fd, dmabuf->file);
+		}
+
 		break;
 	default:
 		ret = -ENOTTY;
 		goto err;
 	}
 
-	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)
-		ret = -EFAULT;
 err:
 	if (kdata != stack_kdata)
 		kfree(kdata);
-- 
2.34.1

Re: [PATCH v2] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds

[Christian König]

On 7/10/26 12:57, Baineng Shou wrote:
> DMA_HEAP_IOCTL_ALLOC allocates a dma-buf and installs an fd into the
> caller's fd table via dma_buf_fd() -> fd_install() before
> dma_heap_ioctl() copies the result back to userspace.  If the trailing
> copy_to_user() fails, userspace never learns the fd number, but the
> fd (and the underlying dma-buf reference) are already visible to
> other threads in the same process and are leaked for the lifetime of
> the process.
> 
> The obvious "close it on the failure path" fix is unsafe: once
> fd_install() has run, another thread can already dup() the fd, send
> it via SCM_RIGHTS, or close() it and let its number be reused, so a
> subsequent close_fd() from the ioctl path can operate on an unrelated
> file.  This was pointed out by Christian König on v1 [1].

IIRC it was Greg who pointed that out numerous times, I'm just repeating what I was told.

> 
> Restructure the allocation path so that fd_install() is the last,
> unfailable step of a successful ioctl:
> 
>   1. heap->ops->allocate()      creates the dma_buf.
>   2. get_unused_fd_flags()      reserves an fd number in the caller's
>                                 fd table without publishing it, so
>                                 no other thread can observe it.
>   3. copy_to_user()             delivers the fd number to userspace;
>                                 on failure the fd is returned with
>                                 put_unused_fd() and the dma_buf
>                                 reference is dropped with
>                                 dma_buf_put(), leaving no user-
>                                 visible state behind.
>   4. fd_install()               publishes the fd -- from here on the
>                                 ioctl cannot fail.
> 
> To make this possible, dma_heap_ioctl_allocate() is refactored to
> return the struct dma_buf * directly (returning ERR_PTR on failure)
> so the caller holds the dmabuf reference across steps 3 and 4.
> The fd is written into the kdata buffer before copy_to_user() so
> the reserved fd number reaches userspace atomically with the install.
> 
> The failure at step 3 is easily reachable from userspace: pass a
> struct dma_heap_allocation_data that lives in a page whose protection
> is flipped to PROT_READ between copy_from_user() and copy_to_user()
> (e.g. via mprotect()).  Before this change each such ioctl leaks one
> dmabuf fd; after it, the fd table is unchanged on failure and only
> /dev/dma_heap/<name> remains open.
> 
> No UAPI or heap-driver interface change.
> 
> [1] https://lore.kernel.org/dri-devel/175e98de-f414-47d7-81c1-c0fe0a8f7f62@amd.com/
> 
> Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework")
> Cc: stable@vger.kernel.org
> Signed-off-by: Baineng Shou <shoubaineng@gmail.com>

Patch looks sane to me, but somebody with more background in DMA-buf heaps should probably take a look as well.

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/dma-buf/dma-heap.c | 80 +++++++++++++++++++-------------------
>  1 file changed, 40 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index a76bf3f8b071..0a9bf62eb06c 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -55,33 +55,6 @@ MODULE_PARM_DESC(mem_accounting,
>  		 "Enable cgroup-based memory accounting for dma-buf heap allocations (default=false).");
>  EXPORT_SYMBOL_NS_GPL(mem_accounting, "DMA_BUF_HEAP");
>  
> -static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> -				 u32 fd_flags,
> -				 u64 heap_flags)
> -{
> -	struct dma_buf *dmabuf;
> -	int fd;
> -
> -	/*
> -	 * Allocations from all heaps have to begin
> -	 * and end on page boundaries.
> -	 */
> -	len = PAGE_ALIGN(len);
> -	if (!len)
> -		return -EINVAL;
> -
> -	dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags);
> -	if (IS_ERR(dmabuf))
> -		return PTR_ERR(dmabuf);
> -
> -	fd = dma_buf_fd(dmabuf, fd_flags);
> -	if (fd < 0) {
> -		dma_buf_put(dmabuf);
> -		/* just return, as put will call release and that will free */
> -	}
> -	return fd;
> -}
> -
>  static int dma_heap_open(struct inode *inode, struct file *file)
>  {
>  	struct dma_heap *heap;
> @@ -99,30 +72,42 @@ static int dma_heap_open(struct inode *inode, struct file *file)
>  	return 0;
>  }
>  
> -static long dma_heap_ioctl_allocate(struct file *file, void *data)
> +static struct dma_buf *dma_heap_ioctl_allocate(struct file *file, void *data)
>  {
>  	struct dma_heap_allocation_data *heap_allocation = data;
>  	struct dma_heap *heap = file->private_data;
> +	struct dma_buf *dmabuf;
>  	int fd;
> +	size_t len;
>  
>  	if (heap_allocation->fd)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
>  
>  	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
> +
> +	len = PAGE_ALIGN(heap_allocation->len);
> +	if (!len)
> +		return ERR_PTR(-EINVAL);
> +
> +	dmabuf = heap->ops->allocate(heap, len, heap_allocation->fd_flags,
> +				     heap_allocation->heap_flags);
>  
> -	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
> -				   heap_allocation->fd_flags,
> -				   heap_allocation->heap_flags);
> -	if (fd < 0)
> -		return fd;
> +	if (IS_ERR(dmabuf))
> +		return dmabuf;
> +
> +	fd = get_unused_fd_flags(heap_allocation->fd_flags);
> +	if (fd < 0) {
> +		dma_buf_put(dmabuf);
> +		return ERR_PTR(fd);
> +	}
>  
>  	heap_allocation->fd = fd;
>  
> -	return 0;
> +	return dmabuf;
>  }
>  
>  static unsigned int dma_heap_ioctl_cmds[] = {
> @@ -138,6 +123,8 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
>  	unsigned int in_size, out_size, drv_size, ksize;
>  	int nr = _IOC_NR(ucmd);
>  	int ret = 0;
> +	int fd;
> +	struct dma_buf *dmabuf;
>  
>  	if (nr >= ARRAY_SIZE(dma_heap_ioctl_cmds))
>  		return -EINVAL;
> @@ -174,15 +161,28 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
>  
>  	switch (kcmd) {
>  	case DMA_HEAP_IOCTL_ALLOC:
> -		ret = dma_heap_ioctl_allocate(file, kdata);
> +		dmabuf = dma_heap_ioctl_allocate(file, kdata);
> +
> +		if (IS_ERR(dmabuf)) {
> +			ret = PTR_ERR(dmabuf);
> +			break;
> +		}
> +
> +		fd = ((struct dma_heap_allocation_data *)kdata)->fd;
> +		if (copy_to_user((void __user *)arg, kdata, out_size) != 0) {
> +			put_unused_fd(fd);
> +			dma_buf_put(dmabuf);
> +			ret = -EFAULT;
> +		} else {
> +			fd_install(fd, dmabuf->file);
> +		}
> +
>  		break;
>  	default:
>  		ret = -ENOTTY;
>  		goto err;
>  	}
>  
> -	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)
> -		ret = -EFAULT;
>  err:
>  	if (kdata != stack_kdata)
>  		kfree(kdata);