dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Baineng Shou <shoubaineng@gmail.com>
To: "Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	"T . J . Mercier" <tjmercier@google.com>,
	"Benjamin Gaignard" <benjamin.gaignard@collabora.com>,
	"Brian Starkey" <Brian.Starkey@arm.com>,
	"John Stultz" <jstultz@google.com>,
	"Sandeep Patil" <sspatil@android.com>,
	"Andrew F . Davis" <afd@ti.com>,
	"Srinivas Kandagatla" <srini@kernel.org>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>
Cc: stable@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	Baineng Shou <shoubaineng@gmail.com>
Subject: [PATCH v5 4/4] selftests: dmabuf-heaps: add fd-leak-on-EFAULT regression test
Date: Thu, 30 Jul 2026 14:26:45 +0800	[thread overview]
Message-ID: <20260730062645.233148-5-shoubaineng@gmail.com> (raw)
In-Reply-To: <20260730062645.233148-1-shoubaineng@gmail.com>

Add a test case that verifies no file descriptor is leaked when
DMA_HEAP_IOCTL_ALLOC succeeds internally but copy_to_user() fails
to deliver the fd number back to userspace.

The failure is triggered by placing the ioctl argument in a private
anonymous page and flipping it to PROT_READ (via mprotect) between
the kernel's copy_from_user() and copy_to_user() calls.  With the
buggy kernel the ioctl returns -EFAULT but leaves an extra open fd
in the process's fd table; with the fixed kernel the fd count is
unchanged.

This serves as a regression test for:
  "dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds"

Suggested-by: Sumit Semwal <sumit.semwal@linaro.org>
Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
---
 .../selftests/dmabuf-heaps/dmabuf-heap.c      | 115 +++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
index fc9694fc4e89..bd58e5b06c8b 100644
--- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
+++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
@@ -390,6 +390,118 @@ static void test_alloc_errors(char *heap_name)
 	close(heap_fd);
 }
 
+/*
+ * test_alloc_no_fd_leak_on_efault - verify no fd is leaked when
+ * copy_to_user() fails during DMA_HEAP_IOCTL_ALLOC.
+ *
+ * The bug: dma_buf_fd() called fd_install() before copy_to_user().
+ * If copy_to_user() then failed (e.g. via mprotect), the fd was
+ * silently installed in the fd table but never returned to userspace.
+ *
+ * The fix: reserve the fd with get_unused_fd_flags() first, attempt
+ * copy_to_user(), and only call fd_install() on success.
+ *
+ * We trigger the failure by placing the ioctl argument in a page,
+ * flipping it to PROT_READ between copy_from_user and copy_to_user,
+ * and counting open file descriptors before and after.
+ */
+static void test_alloc_no_fd_leak_on_efault(char *heap_name)
+{
+	int heap_fd = -1;
+	int fd_before, fd_after;
+	int ret;
+	long page_size;
+	struct dma_heap_allocation_data *req;
+
+	ksft_print_msg("Testing no fd leak when copy_to_user() fails:\n");
+
+	heap_fd = dmabuf_heap_open(heap_name);
+
+	page_size = sysconf(_SC_PAGESIZE);
+
+	/*
+	 * Place the ioctl argument in its own private anonymous page so
+	 * we can flip its protection independently.
+	 */
+	req = mmap(NULL, page_size, PROT_READ | PROT_WRITE,
+		   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (req == MAP_FAILED) {
+		ksft_test_result_fail("mmap failed: %s\n", strerror(errno));
+		goto out;
+	}
+
+	memset(req, 0, sizeof(*req));
+	req->len      = page_size;
+	req->fd_flags = O_RDWR | O_CLOEXEC;
+
+	/* Count open fds before the ioctl */
+	fd_before = 0;
+	{
+		DIR *d = opendir("/proc/self/fd");
+		struct dirent *de;
+
+		if (!d) {
+			ksft_test_result_fail("opendir /proc/self/fd: %s\n",
+					      strerror(errno));
+			munmap(req, page_size);
+			goto out;
+		}
+		while ((de = readdir(d)))
+			if (de->d_name[0] != '.')
+				fd_before++;
+		closedir(d);
+		/* subtract the fd opened by opendir itself */
+	}
+
+	/*
+	 * Make the page read-only: copy_from_user() in the kernel will
+	 * still succeed (it already ran), but copy_to_user() that writes
+	 * the fd number back will fault.
+	 */
+	mprotect(req, page_size, PROT_READ);
+
+	ret = ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, req);
+
+	/* Re-allow writes so munmap can clean up */
+	mprotect(req, page_size, PROT_READ | PROT_WRITE);
+	munmap(req, page_size);
+
+	if (ret != -1 || errno != EFAULT) {
+		/*
+		 * If the ioctl didn't fail with EFAULT, either the kernel
+		 * handled it differently or mprotect raced.  Skip rather
+		 * than giving a false pass/fail.
+		 */
+		ksft_test_result_skip(
+			"ioctl did not return EFAULT (ret=%d errno=%d), skipping\n",
+			ret, errno);
+		goto out;
+	}
+
+	/* Count open fds after the failed ioctl */
+	fd_after = 0;
+	{
+		DIR *d = opendir("/proc/self/fd");
+		struct dirent *de;
+
+		if (!d) {
+			ksft_test_result_fail("opendir /proc/self/fd: %s\n",
+					      strerror(errno));
+			goto out;
+		}
+		while ((de = readdir(d)))
+			if (de->d_name[0] != '.')
+				fd_after++;
+		closedir(d);
+	}
+
+	ksft_test_result(fd_before == fd_after,
+			 "no fd leak on EFAULT: before=%d after=%d\n",
+			 fd_before, fd_after);
+out:
+	close(heap_fd);
+}
+
 static int numer_of_heaps(void)
 {
 	DIR *d = opendir(DEVPATH);
@@ -420,7 +532,7 @@ int main(void)
 		return KSFT_SKIP;
 	}
 
-	ksft_set_plan(11 * numer_of_heaps());
+	ksft_set_plan(12 * numer_of_heaps());
 
 	while ((dir = readdir(d))) {
 		if (!strncmp(dir->d_name, ".", 2))
@@ -435,6 +547,7 @@ int main(void)
 		test_alloc_zeroed(dir->d_name, ONE_MEG);
 		test_alloc_compat(dir->d_name);
 		test_alloc_errors(dir->d_name);
+		test_alloc_no_fd_leak_on_efault(dir->d_name);
 	}
 	closedir(d);
 
-- 
2.34.1


  parent reply	other threads:[~2026-07-30  6:28 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-03  8:09 [PATCH] dma-buf: dma-heap: close installed fd when copy_to_user() fails Baineng Shou
2026-07-03  8:17 ` sashiko-bot
2026-07-03  8:26 ` Christian König
2026-07-10 10:57 ` [PATCH v2] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds Baineng Shou
2026-07-10 11:06   ` sashiko-bot
2026-07-10 20:20     ` T.J. Mercier
2026-07-11  4:18       ` 寿柏能
2026-07-13 23:33         ` T.J. Mercier
2026-07-14 11:46           ` [PATCH v3 0/2] dma-buf: fix fd leak when copy_to_user() fails after fd_install() Baineng Shou
2026-07-14 11:46             ` [PATCH v3 1/2] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds Baineng Shou
2026-07-14 13:13               ` David Laight
2026-07-14 13:38                 ` 寿柏能
2026-07-14 14:33                   ` David Laight
2026-07-15  2:04                     ` 寿柏能
     [not found]                       ` <CAGCp47zxaZsoBKeXz2YdyxbX8QOy_g8dGwnC9gVpJ-Sqng48Qg@mail.gmail.com>
2026-07-28  6:35                         ` Sumit Semwal
2026-07-30  6:25                           ` [PATCH v5 0/4] dma-buf: fix fd leak when copy_to_user() fails after fd_install() Baineng Shou
2026-07-30  6:25                             ` [PATCH v5 1/4] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds Baineng Shou
2026-07-30  6:25                             ` [PATCH v5 2/4] misc: fastrpc: " Baineng Shou
2026-07-30  6:26                           ` [PATCH v5 0/4] dma-buf: fix fd leak when copy_to_user() fails after fd_install() Baineng Shou
2026-07-30  6:26                             ` [PATCH v5 1/4] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds Baineng Shou
2026-07-30  6:26                             ` [PATCH v5 2/4] misc: fastrpc: " Baineng Shou
2026-07-30  6:26                             ` [PATCH v5 3/4] drm/prime: use dma_buf_fd_install() to preserve export tracing Baineng Shou
2026-07-30  6:40                               ` sashiko-bot
2026-07-31 17:14                               ` T.J. Mercier
2026-08-07 10:09                                 ` [PATCH v6 0/4] dma-buf: fix fd leak when copy_to_user() fails after fd_install() Baineng Shou
2026-08-07 10:10                                 ` Baineng Shou
2026-08-07 10:10                                   ` [PATCH v6 1/4] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds Baineng Shou
2026-08-07 10:10                                 ` [PATCH v6 0/4] dma-buf: fix fd leak when copy_to_user() fails after fd_install() Baineng Shou
2026-08-07 10:11                                 ` Baineng Shou
2026-08-07 10:11                                 ` Baineng Shou
2026-08-07 10:11                                   ` [PATCH v6 1/4] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds Baineng Shou
2026-08-07 10:11                                   ` [PATCH v6 2/4] misc: fastrpc: " Baineng Shou
2026-08-07 10:11                                   ` [PATCH v6 3/4] drm/prime: use dma_buf_fd_install() to preserve export tracing Baineng Shou
2026-08-07 10:22                                     ` sashiko-bot
2026-08-07 10:11                                   ` [PATCH v6 4/4] selftests: dmabuf-heaps: add fd-leak-on-EFAULT regression test Baineng Shou
2026-08-07 10:20                                     ` sashiko-bot
2026-08-07 21:49                                     ` T.J. Mercier
2026-07-30  6:26                             ` Baineng Shou [this message]
2026-07-30  6:38                               ` [PATCH v5 " sashiko-bot
2026-07-31 17:09                               ` T.J. Mercier
2026-07-14 11:46             ` [PATCH v3 2/2] misc: fastrpc: don't publish fd before copy_to_user() succeeds Baineng Shou
2026-07-14 12:08               ` sashiko-bot
2026-07-15 20:44               ` T.J. Mercier
2026-07-14 12:24             ` [PATCH v3 0/2] dma-buf: fix fd leak when copy_to_user() fails after fd_install() Christian König
2026-07-14 13:27               ` [PATCH v3] drm/prime: use dma_buf_fd_install() to preserve export tracing Baineng Shou
2026-07-14 13:45                 ` sashiko-bot
2026-07-10 12:33   ` [PATCH v2] dma-buf: dma-heap: don't publish fd before copy_to_user() succeeds Christian König
2026-07-11  4:14 ` Baineng Shou
2026-07-11  4:29   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260730062645.233148-5-shoubaineng@gmail.com \
    --to=shoubaineng@gmail.com \
    --cc=Brian.Starkey@arm.com \
    --cc=afd@ti.com \
    --cc=airlied@gmail.com \
    --cc=benjamin.gaignard@collabora.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jstultz@google.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=srini@kernel.org \
    --cc=sspatil@android.com \
    --cc=stable@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tjmercier@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox