Skip to contentSkip to conversation

FAILED: patch "[PATCH] misc: fastrpc: don't publish fd before copy_to_user()" failed to apply to 6.1-stable tree

PATCH New [email protected] 2 participants 3 messages Sep 8 – Sep 10

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x a4a1a2bfcb29785292d634d7787edc6fb550714d
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026090821-regroup-crunching-93c8@gregkh' --subject-prefix 'PATCH 6.1.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From a4a1a2bfcb29785292d634d7787edc6fb550714d Mon Sep 17 00:00:00 2001
From: Baineng Shou <[email protected]>
Date: Mon, 17 Aug 2026 13:04:55 +0800
Subject: [PATCH] misc: fastrpc: don't publish fd before copy_to_user()
succeeds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fastrpc_ioctl_alloc_dmabuf() calls dma_buf_fd() which installs the fd into the caller's fd table before copy_to_user() copies the fd number back to userspace. If copy_to_user() fails, the fd is already visible to other threads in the same process but the ioctl returns -EFAULT. The existing comment in the code even acknowledges the problem:

"The usercopy failed, but we can't do much about it, as dma_buf_fd()
already called fd_install()..."

Now that dma_buf_fd_install() is available (introduced to fix the same issue in dma-heap), apply the same pattern here: reserve the fd with get_unused_fd_flags(), attempt copy_to_user(), and only on success call dma_buf_fd_install() to publish it atomically with the tracepoint. On copy_to_user() failure, put_unused_fd() and dma_buf_put() cleanly unwind without any user-visible side effects.

drivers/misc/fastrpc.c +6 -10
index eb6c2a78d3c7..d7a9e12f5575 100644
@@ -1712,24 +1712,20 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp)
 		return err;
 	}
 
-	bp.fd = dma_buf_fd(buf->dmabuf, O_ACCMODE);
+	bp.fd = get_unused_fd_flags(O_ACCMODE);
 	if (bp.fd < 0) {
 		dma_buf_put(buf->dmabuf);
-		return -EINVAL;
+		return bp.fd;
 	}
 
 	if (copy_to_user(argp, &bp, sizeof(bp))) {
-		/*
-		 * The usercopy failed, but we can't do much about it, as
-		 * dma_buf_fd() already called fd_install() and made the
-		 * file descriptor accessible for the current process. It
-		 * might already be closed and dmabuf no longer valid when
-		 * we reach this point. Therefore "leak" the fd and rely on
-		 * the process exit path to do any required cleanup.
-		 */
+		put_unused_fd(bp.fd);
+		dma_buf_put(buf->dmabuf);
 		return -EFAULT;
 	}
 
+	dma_buf_fd_install(buf->dmabuf, bp.fd);
+
 	return 0;
 }
 
maintainer
#1 lore raw

PATCH 1/2 dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds

From: Baineng Shou <[email protected]>

[ Upstream commit 30d0aff2c65a277135cfd8ea28fa1ee75e0ea4e0 ]

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. dma_buf_fd_install() publishes the fd and emits the
trace_dma_buf_fd tracepoint -- from here on the ioctl cannot fail.

A new dma_buf_fd_install() helper is introduced in dma-buf.c to wrap fd_install() together with the DMA_BUF_TRACE() call, preserving the export tracing that dma_buf_fd() provides. 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 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/[email protected]/

[Backport to 6.6: resolve removal of dma_heap_buffer_alloc() against the older unsigned-int argument types, without importing the newer mem_accounting module parameter. Keep the allocation, fd reservation, usercopy and publication ordering in the existing ioctl functions.

This tree has no DMA_BUF_TRACE/trace_dma_buf_fd infrastructure. Replace the new dma_buf_fd_install() function, declaration and symbol export with a single-evaluation macro around the existing fd_install(). This retains the interface required by a4a1a2bfcb29 ("misc: fastrpc: don't publish fd before copy_to_user() succeeds") without adding functions or tracing infrastructure. The dma-buf.c hunk is therefore dropped.]

[ sashal: Reduced backport -- upstream 30d0aff2c65a2 touches 3 file(s), this
backport carries 2. Not backported here:
drivers/dma-buf/dma-buf.c
This note is generated from the file lists only; see the resolution record for the reasoning. ]

Stable-dep-of: a4a1a2bfcb29 ("misc: fastrpc: don't publish fd before copy_to_user() succeeds")

---
 drivers/dma-buf/dma-heap.c | 80 +++++++++++++++++++-------------------
 include/linux/dma-buf.h    |  4 ++
 2 files changed, 44 insertions(+), 40 deletions(-)
drivers/dma-buf/dma-heap.c +40 -40
index 59d158873f4cb..38b425c8e1119 100644
@@ -49,33 +49,6 @@ static dev_t dma_heap_devt;
 static struct class *dma_heap_class;
 static DEFINE_XARRAY_ALLOC(dma_heap_minors);
 
-static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
-				 unsigned int fd_flags,
-				 unsigned int 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;
@@ -93,30 +66,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[] = {
@@ -132,6 +117,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;
@@ -168,15 +155,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 {
+			dma_buf_fd_install(dmabuf, fd);
+		}
+
 		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);
include/linux/dma-buf.h +4 -0
index 9c31f1f430d8e..d0991ef660c3a 100644
@@ -615,6 +615,10 @@ void dma_buf_unpin(struct dma_buf_attachment *attach);
 struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info);
 
 int dma_buf_fd(struct dma_buf *dmabuf, int flags);
+
+/* Publish a reserved fd only after all fallible work has succeeded. */
+#define dma_buf_fd_install(dmabuf, fd) fd_install((fd), (dmabuf)->file)
+
 struct dma_buf *dma_buf_get(int fd);
 void dma_buf_put(struct dma_buf *dmabuf);
 
Signature · 1 line
2.53.0
maintainer
#2 lore raw

PATCH 2/2 misc: fastrpc: don't publish fd before copy_to_user() succeeds

From: Baineng Shou <[email protected]>

[ Upstream commit a4a1a2bfcb29785292d634d7787edc6fb550714d ]

fastrpc_ioctl_alloc_dmabuf() calls dma_buf_fd() which installs the fd into the caller's fd table before copy_to_user() copies the fd number back to userspace. If copy_to_user() fails, the fd is already visible to other threads in the same process but the ioctl returns -EFAULT. The existing comment in the code even acknowledges the problem:

"The usercopy failed, but we can't do much about it, as dma_buf_fd()
already called fd_install()..."

Now that dma_buf_fd_install() is available (introduced to fix the same issue in dma-heap), apply the same pattern here: reserve the fd with get_unused_fd_flags(), attempt copy_to_user(), and only on success call dma_buf_fd_install() to publish it atomically with the tracepoint. On copy_to_user() failure, put_unused_fd() and dma_buf_put() cleanly unwind without any user-visible side effects.

---
 drivers/misc/fastrpc.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/misc/fastrpc.c +6 -10
index 6ea071c109802..15e563238ddc5 100644
@@ -1485,24 +1485,20 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp)
 		return err;
 	}
 
-	bp.fd = dma_buf_fd(buf->dmabuf, O_ACCMODE);
+	bp.fd = get_unused_fd_flags(O_ACCMODE);
 	if (bp.fd < 0) {
 		dma_buf_put(buf->dmabuf);
-		return -EINVAL;
+		return bp.fd;
 	}
 
 	if (copy_to_user(argp, &bp, sizeof(bp))) {
-		/*
-		 * The usercopy failed, but we can't do much about it, as
-		 * dma_buf_fd() already called fd_install() and made the
-		 * file descriptor accessible for the current process. It
-		 * might already be closed and dmabuf no longer valid when
-		 * we reach this point. Therefore "leak" the fd and rely on
-		 * the process exit path to do any required cleanup.
-		 */
+		put_unused_fd(bp.fd);
+		dma_buf_put(buf->dmabuf);
 		return -EFAULT;
 	}
 
+	dma_buf_fd_install(buf->dmabuf, bp.fd);
+
 	return 0;
 }
 
Signature · 1 line
2.53.0