[PATCH 0/1] udmabuf: reject unknown UDMABUF_CREATE flags

[PATCH 0/1] udmabuf: reject unknown UDMABUF_CREATE flags

[Iván Ezequiel Rodriguez]

UDMABUF_CREATE and UDMABUF_CREATE_LIST only define UDMABUF_FLAGS_CLOEXEC
in include/uapi/linux/udmabuf.h. Today the driver masks unknown flag bits
and returns a dmabuf fd anyway, so userspace cannot tell that an
unsupported flag was ignored. dma-heap already rejects unknown fd_flags
and heap_flags with -EINVAL; apply the same policy in udmabuf_create(),
which is shared by both ioctls.

This series adds the driver check, documents the flag semantics in the
uapi header, and extends the existing kselftest with test-unknown-flags
(UDMABUF_FLAGS_CLOEXEC | 0x2 must fail).

Tested with the udmabuf kselftest on a kernel built with this patch
(test-unknown-flags PASS).

Iván Ezequiel Rodriguez (1):
  udmabuf: reject unknown UDMABUF_CREATE flags

 drivers/dma-buf/udmabuf.c                         |  3 +++
 include/uapi/linux/udmabuf.h                      | 13 +++++++++++++
 tools/testing/selftests/drivers/dma-buf/udmabuf.c | 14 +++++++++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/1] udmabuf: reject unknown UDMABUF_CREATE flags

[Iván Ezequiel Rodriguez]

UDMABUF_CREATE and UDMABUF_CREATE_LIST only define
UDMABUF_FLAGS_CLOEXEC. Unknown flag bits were silently masked
before returning a dmabuf fd, so userspace could not detect
unsupported flags. Reject them with -EINVAL, consistent with
dma-heap flag validation.

Document the flag semantics in udmabuf.h and add a selftest.

Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com>
---
 drivers/dma-buf/udmabuf.c                         |  3 +++
 include/uapi/linux/udmabuf.h                      | 13 +++++++++++++
 tools/testing/selftests/drivers/dma-buf/udmabuf.c | 14 +++++++++++++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index bced421c0d65..fb36c0d9ffe7 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -358,6 +358,9 @@ static long udmabuf_create(struct miscdevice *device,
 	long ret = -EINVAL;
 	u32 i, flags;
 
+	if (head->flags & ~UDMABUF_FLAGS_CLOEXEC)
+		return -EINVAL;
+
 	ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
 	if (!ubuf)
 		return -ENOMEM;
diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h
index 46b6532ed855..e267856dd510 100644
--- a/include/uapi/linux/udmabuf.h
+++ b/include/uapi/linux/udmabuf.h
@@ -7,6 +7,13 @@
 
 #define UDMABUF_FLAGS_CLOEXEC	0x01
 
+/**
+ * struct udmabuf_create - parameters for %UDMABUF_CREATE
+ * @memfd: memfd containing the memory to export
+ * @flags: %UDMABUF_FLAGS_CLOEXEC only; unknown bits are rejected
+ * @offset: byte offset into @memfd (page aligned)
+ * @size: number of bytes to export (page aligned, non-zero)
+ */
 struct udmabuf_create {
 	__u32 memfd;
 	__u32 flags;
@@ -21,6 +28,12 @@ struct udmabuf_create_item {
 	__u64 size;
 };
 
+/**
+ * struct udmabuf_create_list - parameters for %UDMABUF_CREATE_LIST
+ * @flags: %UDMABUF_FLAGS_CLOEXEC only; unknown bits are rejected
+ * @count: number of entries in @list (non-zero, capped by driver)
+ * @list: flexible array of memfd regions to combine
+ */
 struct udmabuf_create_list {
 	__u32 flags;
 	__u32 count;
diff --git a/tools/testing/selftests/drivers/dma-buf/udmabuf.c b/tools/testing/selftests/drivers/dma-buf/udmabuf.c
index d78aec662586..95c781855404 100644
--- a/tools/testing/selftests/drivers/dma-buf/udmabuf.c
+++ b/tools/testing/selftests/drivers/dma-buf/udmabuf.c
@@ -138,7 +138,7 @@ int main(int argc, char *argv[])
 	void *addr1, *addr2;
 
 	ksft_print_header();
-	ksft_set_plan(7);
+	ksft_set_plan(8);
 
 	devfd = open("/dev/udmabuf", O_RDWR);
 	if (devfd < 0) {
@@ -189,6 +189,18 @@ int main(int argc, char *argv[])
 	else
 		ksft_test_result_pass("%s: [PASS,test-2]\n", TEST_PREFIX);
 
+	/* should fail (unknown flags) */
+	create.memfd  = memfd;
+	create.offset = 0;
+	create.size   = size;
+	create.flags  = UDMABUF_FLAGS_CLOEXEC | 0x2;
+	buf = ioctl(devfd, UDMABUF_CREATE, &create);
+	if (buf >= 0)
+		ksft_test_result_fail("%s: [FAIL,test-unknown-flags]\n", TEST_PREFIX);
+	else
+		ksft_test_result_pass("%s: [PASS,test-unknown-flags]\n", TEST_PREFIX);
+	create.flags = 0;
+
 	/* should fail (not memfd) */
 	create.memfd  = 0; /* stdin */
 	create.offset = 0;
-- 
2.43.0

Re: [PATCH 1/1] udmabuf: reject unknown UDMABUF_CREATE flags

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Integer overflow in `lsize` calculation in `udmabuf_ioctl_create_list` allows out-of-bounds heap memory access if `list_limit` is misconfigured.
--

commit 0bd9a7c7e5bbe2cbc29a0cdc4dcdb2763d2b20e1
Author: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com>

udmabuf: reject unknown UDMABUF_CREATE flags

This patch updates udmabuf_create() to reject unknown flags by checking
against UDMABUF_FLAGS_CLOEXEC and returning -EINVAL. It also documents
the flag semantics in udmabuf.h and adds a selftest.

> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index bced421c0d658..fb36c0d9ffe72 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -358,6 +358,9 @@ static long udmabuf_create(struct miscdevice *device,
>  	long ret = -EINVAL;
>  	u32 i, flags;
>  
> +	if (head->flags & ~UDMABUF_FLAGS_CLOEXEC)
> +		return -EINVAL;
> +
[Severity: High]
This is a pre-existing issue, but can this code trigger an out-of-bounds
heap read if the list limit is misconfigured?

If an administrator sets the module parameter list_limit to a negative
value (like -1 to mean unlimited), a large head.count from userspace
can bypass the limit check in udmabuf_ioctl_create_list() because
head.count is an unsigned u32 and list_limit is promoted to unsigned
for the comparison.

If a user calls ioctl(UDMABUF_CREATE_LIST) with a crafted head.count
of 0x20000001, the allocation size calculation in
udmabuf_ioctl_create_list() overflows:

drivers/dma-buf/udmabuf.c:udmabuf_ioctl_create_list() {
    ...
    lsize = sizeof(struct udmabuf_create_item) * head.count;
    list = memdup_user((void __user *)(arg + sizeof(head)), lsize);
}

The size calculation evaluates to 0x300000018, which is truncated to 24
bytes when assigned to the u32 lsize variable. memdup_user() then
allocates a 24-byte buffer for the list.

When execution reaches udmabuf_create(), the loop iterates head->count
times:

drivers/dma-buf/udmabuf.c:udmabuf_create() {
    ...
    for (i = 0; i < head->count; i++) {
        pgoff_t subpgcnt;

        if (!PAGE_ALIGNED(list[i].offset))
            goto err_noinit;
    ...
}

Since the list buffer is only 24 bytes long but the loop iterates
0x20000001 times, does this result in out-of-bounds reads on the kernel
heap that could trigger a page fault and kernel oops?

>  	ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
>  	if (!ubuf)
>  		return -ENOMEM;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701172159.270872-2-ivanrwcm25@gmail.com?part=1