[PATCH v2 0/5] ntsync miscellaneous patches
This is a resend of a series originally submitted as [1], combined with two patches originally sent as [2]. This supersedes a previous set of patches sent as [3]. The following is the original cover letter sent with the series at [1]. [1] https://lore.kernel.org/all/20260628024239.152852-1-ivanrwcm25@gmail.com/ [2] https://lore.kernel.org/all/20260720175402.44338-1-ivanrwcm25@gmail.com/ [3] https://lore.kernel.org/all/20260720171740.447035-1-zfigura@codeweavers.com/ === This series improves ntsync without changing wait/wake semantics: 1/4 — Align Documentation/userspace-api/ntsync.rst with include/uapi/linux/ntsync.h (ioctl macro names and struct layout). 2/4 — Fix wake_all selftest: CREATE_EVENT returns an fd, not zero. 3/4 — Add selftests for documented EINVAL cases (zero owner, non-zero pad, cross-instance object use). 4/4 — Reject wait ioctls when owner is zero, matching the documented uAPI (3/4 depends on 4/4 for the owner tests). Patch 4/4 closes a spec gap: Documentation/userspace-api/ntsync.rst requires EINVAL when wait owner is zero, but setup_wait() only validated pad and flags. Unlock/kill mutex ioctls already reject owner == 0. Testing: - scripts/checkpatch.pl --strict --no-tree: clean (4/4 patches) - make headers && make -C tools/testing/selftests TARGETS=drivers/ntsync - Kernel 7.1.0-ntsync-test+ (CONFIG_NTSYNC=y), QEMU x86_64 initramfs: tools/testing/selftests/drivers/ntsync/ntsync — 12/12 PASS, including wake_all and wait_args_validation - On 6.17.0-35-generic with the distro ntsync.ko (without patch 4/4): wait_args_validation fails on owner==0 (wait proceeds instead of EINVAL), confirming the gap this series fixes Iván Ezequiel Rodriguez (5): selftests: ntsync: fix wake_all CREATE_EVENT fd expectation selftests: ntsync: add wait argument validation tests ntsync: reject wait ioctls with zero owner docs: ntsync: align ioctl names and struct layouts with uapi selftests: ntsync: test absolute MONOTONIC waits under time namespaces Documentation/userspace-api/ntsync.rst | 33 ++--- drivers/misc/ntsync.c | 3 + .../testing/selftests/drivers/ntsync/ntsync.c | 133 +++++++++++++++++- 3 files changed, 152 insertions(+), 17 deletions(-) base-commit: 2cedf2272f1bb42471e646868ac572cc5752bd91 -- 2.53.0
From: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> wake_all used EXPECT_EQ(0, objs[3]) after NTSYNC_IOC_CREATE_EVENT. The ioctl returns a non-negative file descriptor on success; check EXPECT_LE(0, objs[3]) like the other CREATE_* paths. The incorrect expectation was noted on list (Mar 2025) but is still present in mainline. Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com> --- tools/testing/selftests/drivers/ntsync/ntsync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c index e6a37214aa46..12b4b81edf7f 100644 --- a/tools/testing/selftests/drivers/ntsync/ntsync.c +++ b/tools/testing/selftests/drivers/ntsync/ntsync.c @@ -968,7 +968,7 @@ TEST(wake_all) auto_event_args.manual = false; auto_event_args.signaled = true; objs[3] = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &auto_event_args); - EXPECT_EQ(0, objs[3]); + EXPECT_LE(0, objs[3]); wait_args.timeout = get_abs_timeout(1000); wait_args.objs = (uintptr_t)objs; -- 2.53.0
From: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> Add coverage for documented EINVAL cases: zero owner on wait any/all, non-zero pad, and objects from a different /dev/ntsync instance. Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com> --- .../testing/selftests/drivers/ntsync/ntsync.c | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c index 12b4b81edf7f..c9fe4d5987ec 100644 --- a/tools/testing/selftests/drivers/ntsync/ntsync.c +++ b/tools/testing/selftests/drivers/ntsync/ntsync.c @@ -1340,4 +1340,48 @@ TEST(stress_wait) close(stress_device); } +TEST(wait_args_validation) +{ + struct ntsync_sem_args sem_args = { .count = 1, .max = 1 }; + struct ntsync_wait_args wait_args = {0}; + struct timespec timeout; + int fd, fd2, sem, ret; + __u32 index; + + fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY); + ASSERT_GE(fd, 0); + + fd2 = open("/dev/ntsync", O_CLOEXEC | O_RDONLY); + ASSERT_GE(fd2, 0); + + sem = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args); + EXPECT_GE(sem, 0); + + ret = wait_any(fd, 1, &sem, 0, &index); + EXPECT_EQ(-1, ret); + EXPECT_EQ(EINVAL, errno); + + ret = wait_all(fd, 1, &sem, 0, &index); + EXPECT_EQ(-1, ret); + EXPECT_EQ(EINVAL, errno); + + clock_gettime(CLOCK_MONOTONIC, &timeout); + wait_args.timeout = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec; + wait_args.count = 0; + wait_args.objs = 0; + wait_args.owner = 123; + wait_args.pad = 1; + ret = ioctl(fd, NTSYNC_IOC_WAIT_ANY, &wait_args); + EXPECT_EQ(-1, ret); + EXPECT_EQ(EINVAL, errno); + + ret = wait_any(fd2, 1, &sem, 123, &index); + EXPECT_EQ(-1, ret); + EXPECT_EQ(EINVAL, errno); + + close(sem); + close(fd2); + close(fd); +} + TEST_HARNESS_MAIN -- 2.53.0
From: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> setup_wait() already validates pad and flags but not owner, while Documentation/userspace-api/ntsync.rst requires EINVAL when owner is zero. Reject early before queueing waiters. Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com> --- drivers/misc/ntsync.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c index 02c9d1192812..4a805919bb0c 100644 --- a/drivers/misc/ntsync.c +++ b/drivers/misc/ntsync.c @@ -875,6 +875,9 @@ static int setup_wait(struct ntsync_device *dev, if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME)) return -EINVAL; + if (!args->owner) + return -EINVAL; + if (size >= sizeof(fds)) return -EINVAL; -- 2.53.0
From: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> The uAPI documentation used outdated or conceptual ioctl macro names and listed ntsync_event_args / ntsync_wait_args fields in the wrong order. Match include/uapi/linux/ntsync.h and note that absolute MONOTONIC timeouts are interpreted in the caller's time namespace. Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com> --- Documentation/userspace-api/ntsync.rst | 33 +++++++++++++------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/Documentation/userspace-api/ntsync.rst b/Documentation/userspace-api/ntsync.rst index 25e7c4aef968..a778016feb87 100644 --- a/Documentation/userspace-api/ntsync.rst +++ b/Documentation/userspace-api/ntsync.rst @@ -83,18 +83,18 @@ structures used in ioctl calls:: }; struct ntsync_event_args { - __u32 signaled; __u32 manual; + __u32 signaled; }; struct ntsync_wait_args { __u64 timeout; __u64 objs; __u32 count; - __u32 owner; __u32 index; - __u32 alert; __u32 flags; + __u32 owner; + __u32 alert; __u32 pad; }; @@ -152,7 +152,7 @@ The ioctls on the device file are as follows: The ioctls on the individual objects are as follows: -.. c:macro:: NTSYNC_IOC_SEM_POST +.. c:macro:: NTSYNC_IOC_SEM_RELEASE Post to a semaphore object. Takes a pointer to a 32-bit integer, which on input holds the count to be added to the semaphore, and on @@ -186,7 +186,7 @@ The ioctls on the individual objects are as follows: unowned and signaled, and eligible threads waiting on it will be woken as appropriate. -.. c:macro:: NTSYNC_IOC_SET_EVENT +.. c:macro:: NTSYNC_IOC_EVENT_SET Signal an event object. Takes a pointer to a 32-bit integer, which on output contains the previous state of the event. @@ -194,12 +194,12 @@ The ioctls on the individual objects are as follows: Eligible threads will be woken, and auto-reset events will be designaled appropriately. -.. c:macro:: NTSYNC_IOC_RESET_EVENT +.. c:macro:: NTSYNC_IOC_EVENT_RESET Designal an event object. Takes a pointer to a 32-bit integer, which on output contains the previous state of the event. -.. c:macro:: NTSYNC_IOC_PULSE_EVENT +.. c:macro:: NTSYNC_IOC_EVENT_PULSE Wake threads waiting on an event object while leaving it in an unsignaled state. Takes a pointer to a 32-bit integer, which on @@ -213,7 +213,7 @@ The ioctls on the individual objects are as follows: afterwards, and a simultaneous read operation will always report the event as unsignaled. -.. c:macro:: NTSYNC_IOC_READ_SEM +.. c:macro:: NTSYNC_IOC_SEM_READ Read the current state of a semaphore object. Takes a pointer to struct :c:type:`ntsync_sem_args`, which is used as follows: @@ -225,7 +225,7 @@ The ioctls on the individual objects are as follows: * - ``max`` - On output, contains the maximum count of the semaphore. -.. c:macro:: NTSYNC_IOC_READ_MUTEX +.. c:macro:: NTSYNC_IOC_MUTEX_READ Read the current state of a mutex object. Takes a pointer to struct :c:type:`ntsync_mutex_args`, which is used as follows: @@ -242,7 +242,7 @@ The ioctls on the individual objects are as follows: ``EOWNERDEAD``. In this case, ``count`` and ``owner`` are set to zero. -.. c:macro:: NTSYNC_IOC_READ_EVENT +.. c:macro:: NTSYNC_IOC_EVENT_READ Read the current state of an event object. Takes a pointer to struct :c:type:`ntsync_event_args`, which is used as follows: @@ -255,7 +255,7 @@ The ioctls on the individual objects are as follows: - On output, contains 1 if the event is a manual-reset event, and 0 otherwise. -.. c:macro:: NTSYNC_IOC_KILL_OWNER +.. c:macro:: NTSYNC_IOC_MUTEX_KILL Mark a mutex as unowned and abandoned if it is owned by the given owner. Takes an input-only pointer to a 32-bit integer denoting the @@ -276,11 +276,12 @@ The ioctls on the individual objects are as follows: * - ``timeout`` - Absolute timeout in nanoseconds. If ``NTSYNC_WAIT_REALTIME`` is set, the timeout is measured against the REALTIME clock; - otherwise it is measured against the MONOTONIC clock. If the - timeout is equal to or earlier than the current time, the - function returns immediately without sleeping. If ``timeout`` - is U64_MAX, the function will sleep until an object is - signaled, and will not fail with ``ETIMEDOUT``. + otherwise it is measured against the MONOTONIC clock in the + caller's time namespace. If the timeout is equal to or earlier + than the current time, the function returns immediately + without sleeping. If ``timeout`` is U64_MAX, the function will + sleep until an object is signaled, and will not fail with + ``ETIMEDOUT``. * - ``objs`` - Pointer to an array of ``count`` file descriptors (specified as an integer so that the structure has the same -- 2.53.0
From: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> Cover the timens conversion in ntsync_schedule(): with a negative CLOCK_MONOTONIC offset, a 100 ms absolute wait must not return immediately against the host clock. Suggested-by: Maoyi Xie <maoyixie.tju@gmail.com> Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com> Reviewed-by: Elizabeth Figura <zfigura@codeweavers.com> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com> --- .../testing/selftests/drivers/ntsync/ntsync.c | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c index c9fe4d5987ec..1f0dc43bb4c0 100644 --- a/tools/testing/selftests/drivers/ntsync/ntsync.c +++ b/tools/testing/selftests/drivers/ntsync/ntsync.c @@ -8,12 +8,18 @@ #define _GNU_SOURCE #include <sys/ioctl.h> #include <sys/stat.h> +#include <sys/wait.h> #include <fcntl.h> +#include <sched.h> #include <time.h> #include <pthread.h> #include <linux/ntsync.h> #include "kselftest_harness.h" +#ifndef CLONE_NEWTIME +#define CLONE_NEWTIME 0x00000080 +#endif + static int read_sem_state(int sem, __u32 *count, __u32 *max) { struct ntsync_sem_args args; @@ -1384,4 +1390,85 @@ TEST(wait_args_validation) close(fd); } +/* + * Absolute MONOTONIC timeouts must honour the caller's time namespace. + * With a negative monotonic offset, a 100 ms wait must still take ~100 ms + * of namespace time (not return immediately against the host clock). + */ +TEST(wait_any_monotonic_timens) +{ + struct ntsync_sem_args sem_args = {0}; + struct ntsync_wait_args wait_args = {0}; + struct timespec start, end; + char buf[64]; + __u64 elapsed_ns; + int fd, offset_fd, sem, ret, status, len; + pid_t pid; + + if (access("/proc/self/ns/time", F_OK)) + SKIP(return, "Time namespaces are not supported"); + + fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY); + if (fd < 0) + SKIP(return, "/dev/ntsync is not available"); + + ret = unshare(CLONE_NEWTIME); + if (ret) { + close(fd); + if (errno == EPERM) + SKIP(return, "need CAP_SYS_ADMIN for CLONE_NEWTIME"); + ASSERT_EQ(0, ret); + } + + len = snprintf(buf, sizeof(buf), "%d %d 0", CLOCK_MONOTONIC, -10); + offset_fd = open("/proc/self/timens_offsets", O_WRONLY); + ASSERT_LE(0, offset_fd); + ASSERT_EQ(len, write(offset_fd, buf, len)); + close(offset_fd); + + pid = fork(); + ASSERT_LE(0, pid); + if (!pid) { + int obj; + + sem_args.count = 0; + sem_args.max = 1; + sem = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args); + if (sem < 0) + _exit(1); + + obj = sem; + wait_args.timeout = get_abs_timeout(100); + wait_args.objs = (uintptr_t)&obj; + wait_args.count = 1; + wait_args.owner = 123; + wait_args.index = 0xdeadbeef; + + if (clock_gettime(CLOCK_MONOTONIC, &start)) + _exit(2); + ret = ioctl(fd, NTSYNC_IOC_WAIT_ANY, &wait_args); + if (clock_gettime(CLOCK_MONOTONIC, &end)) + _exit(2); + + if (ret != -1 || errno != ETIMEDOUT) + _exit(3); + + elapsed_ns = (end.tv_sec - start.tv_sec) * 1000000000ULL + + (end.tv_nsec - start.tv_nsec); + /* Without timens conversion this returns in ~0 ms. */ + if (elapsed_ns < 50 * 1000000ULL) + _exit(4); + if (elapsed_ns > 1000 * 1000000ULL) + _exit(5); + + _exit(0); + } + + ASSERT_EQ(pid, waitpid(pid, &status, 0)); + EXPECT_TRUE(WIFEXITED(status)); + EXPECT_EQ(0, WEXITSTATUS(status)); + + close(fd); +} + TEST_HARNESS_MAIN -- 2.53.0
participants (1)
-
Elizabeth Figura