All of 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 v6 4/4] selftests: dmabuf-heaps: add fd-leak-on-EFAULT regression test
Date: Fri,  7 Aug 2026 18:11:40 +0800	[thread overview]
Message-ID: <20260807101140.1357218-5-shoubaineng@gmail.com> (raw)
In-Reply-To: <20260807101140.1357218-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      | 113 +++++++++++++++++-
 1 file changed, 112 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..1d49df671919 100644
--- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
+++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
@@ -390,6 +390,116 @@ static void test_alloc_errors(char *heap_name)
 	close(heap_fd);
 }
 
+/*
+ * count_open_fds - return the number of open file descriptors.
+ *
+ * The fd opened by opendir() itself is counted, but since it is opened
+ * and closed within each call, it cancels out when comparing two counts.
+ * Returns -1 on error.
+ */
+static int count_open_fds(void)
+{
+	DIR *d = opendir("/proc/self/fd");
+	struct dirent *de;
+	int count = 0;
+
+	if (!d)
+		return -1;
+
+	while ((de = readdir(d)))
+		if (de->d_name[0] != '.')
+			count++;
+	closedir(d);
+	return count;
+}
+
+/*
+ * 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 private
+ * anonymous page and flipping it to PROT_READ before the ioctl.
+ * Inside the kernel, copy_from_user() reads from the page (reads are
+ * allowed under PROT_READ, so it succeeds), but copy_to_user() that
+ * writes the fd number back faults, returning -EFAULT.  We then
+ * count open file descriptors before and after; with the bug an extra
+ * fd is left in the table.
+ */
+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 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;
+
+	fd_before = count_open_fds();
+	if (fd_before < 0) {
+		ksft_test_result_fail("count_open_fds: %s\n", strerror(errno));
+		munmap(req, page_size);
+		goto out;
+	}
+
+	/*
+	 * Make the page read-only so copy_to_user() will fault.  The
+	 * ioctl must fail with -1; if it returns success the test setup
+	 * is broken (mprotect is synchronous, so there is no race).
+	 */
+	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) {
+		ksft_test_result_fail("ioctl returned %d, expected -1 EFAULT\n",
+				      ret);
+		goto out;
+	}
+
+	fd_after = count_open_fds();
+	if (fd_after < 0) {
+		ksft_test_result_fail("count_open_fds: %s\n", strerror(errno));
+		goto out;
+	}
+
+	ksft_test_result(fd_before == fd_after,
+			 "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 +530,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 +545,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-08-07 10:12 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                                   ` Baineng Shou [this message]
2026-08-07 10:20                                     ` [PATCH v6 4/4] selftests: dmabuf-heaps: add fd-leak-on-EFAULT regression test sashiko-bot
2026-08-07 21:49                                     ` T.J. Mercier
2026-07-30  6:26                             ` [PATCH v5 " Baineng Shou
2026-07-30  6:38                               ` 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=20260807101140.1357218-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.