Re: [PATCH v6 4/4] selftests: dmabuf-heaps: add fd-leak-on-EFAULT regression test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The regression test silently reports a false pass if the ioctl fails for a reason other than the expected -EFAULT.
- [Low] A file descriptor is leaked on the test's failure path if the ioctl unexpectedly succeeds.
--

--- Patch [4]: [PATCH v6 4/4] selftests: dmabuf-heaps: add fd-leak-on-EFAULT regression test ---
commit 2ee174de4cce870677ff17aa1714b40304934392
Author: Baineng Shou <shoubaineng@gmail.com>

selftests: dmabuf-heaps: add fd-leak-on-EFAULT regression test

This commit adds 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. It triggers the failure by placing the ioctl
argument in a private anonymous page and changing it to PROT_READ between
the copy_from_user() and copy_to_user() calls.

> diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> index fc9694fc4e89e..1d49df6719198 100644
> --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
[ ... ]
> +	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);
> +

[Severity: Low]
Does this code leak the newly allocated file descriptor if the ioctl
unexpectedly succeeds?

Since the test unmaps the req structure immediately after the ioctl, the
returned file descriptor stored in req->fd is lost. If the ioctl returned 0,
the fd cannot be read and closed before the test jumps to the error path.

> +	if (ret != -1) {

[Severity: Medium]
Will this silently report a false pass if the ioctl fails for a reason other
than the expected EFAULT?

The test checks that the return value is -1, but doesn't verify that
errno == EFAULT. If the ioctl fails early (for example, returning -EINVAL
or -ENOMEM) before reaching the copy_to_user() step, the fd count remains
unchanged and the test passes, even though the EFAULT path was never tested.

> +		ksft_test_result_fail("ioctl returned %d, expected -1 EFAULT\n",
> +				      ret);
> +		goto out;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807101140.1357218-1-shoubaineng@gmail.com?part=4