Linux userland API discussions
 help / color / mirror / Atom feed
* [PATCH 23/31] selftests: ntsync: Add some tests for wakeup signaling with WINESYNC_IOC_WAIT_ALL.
From: Elizabeth Figura @ 2024-02-14 23:52 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Test contended "wait-for-all" waits, to make sure that scheduling and wakeup
logic works correctly, and that the wait only exits once objects are all
simultaneously signaled.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 .../testing/selftests/drivers/ntsync/ntsync.c | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index e825e115f11f..063c0ecb3bff 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -706,4 +706,102 @@ TEST(wake_any)
 	close(fd);
 }
 
+TEST(wake_all)
+{
+	struct ntsync_mutex_args mutex_args = {0};
+	struct ntsync_wait_args wait_args = {0};
+	struct ntsync_sem_args sem_args = {0};
+	struct wait_args thread_args;
+	int objs[2], fd, ret;
+	__u32 count, index;
+	pthread_t thread;
+
+	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+	ASSERT_LE(0, fd);
+
+	sem_args.count = 0;
+	sem_args.max = 3;
+	sem_args.sem = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, sem_args.sem);
+
+	mutex_args.owner = 123;
+	mutex_args.count = 1;
+	mutex_args.mutex = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+
+	objs[0] = sem_args.sem;
+	objs[1] = mutex_args.mutex;
+
+	wait_args.timeout = get_abs_timeout(1000);
+	wait_args.objs = (uintptr_t)objs;
+	wait_args.count = 2;
+	wait_args.owner = 456;
+	thread_args.fd = fd;
+	thread_args.args = &wait_args;
+	thread_args.request = NTSYNC_IOC_WAIT_ALL;
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	count = 1;
+	ret = post_sem(sem_args.sem, &count);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, count);
+
+	ret = pthread_tryjoin_np(thread, NULL);
+	EXPECT_EQ(EBUSY, ret);
+
+	check_sem_state(sem_args.sem, 1, 3);
+
+	ret = wait_any(fd, 1, &sem_args.sem, 123, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+
+	ret = unlock_mutex(mutex_args.mutex, 123, &count);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, count);
+
+	ret = pthread_tryjoin_np(thread, NULL);
+	EXPECT_EQ(EBUSY, ret);
+
+	check_mutex_state(mutex_args.mutex, 0, 0);
+
+	count = 2;
+	ret = post_sem(sem_args.sem, &count);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, count);
+	check_sem_state(sem_args.sem, 1, 3);
+	check_mutex_state(mutex_args.mutex, 1, 456);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, thread_args.ret);
+
+	/* delete an object while it's being waited on */
+
+	wait_args.timeout = get_abs_timeout(200);
+	wait_args.owner = 123;
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	close(sem_args.sem);
+	close(mutex_args.mutex);
+
+	ret = wait_for_thread(thread, 200);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(-1, thread_args.ret);
+	EXPECT_EQ(ETIMEDOUT, thread_args.err);
+
+	close(fd);
+}
+
 TEST_HARNESS_MAIN
-- 
2.43.0


^ permalink raw reply related

* [PATCH 24/31] selftests: ntsync: Add some tests for manual-reset event state.
From: Elizabeth Figura @ 2024-02-14 23:52 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Test event-specific ioctls NTSYNC_IOC_EVENT_SET, NTSYNC_IOC_EVENT_RESET,
NTSYNC_IOC_EVENT_PULSE, NTSYNC_IOC_EVENT_READ for manual-reset events, and
waiting on manual-reset events.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 .../testing/selftests/drivers/ntsync/ntsync.c | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 063c0ecb3bff..7cc6fdf10abd 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -73,6 +73,27 @@ static int unlock_mutex(int mutex, __u32 owner, __u32 *count)
 	return ret;
 }
 
+static int read_event_state(int event, __u32 *signaled, __u32 *manual)
+{
+	struct ntsync_event_args args;
+	int ret;
+
+	memset(&args, 0xcc, sizeof(args));
+	ret = ioctl(event, NTSYNC_IOC_EVENT_READ, &args);
+	*signaled = args.signaled;
+	*manual = args.manual;
+	return ret;
+}
+
+#define check_event_state(event, signaled, manual) \
+	({ \
+		__u32 __signaled, __manual; \
+		int ret = read_event_state((event), &__signaled, &__manual); \
+		EXPECT_EQ(0, ret); \
+		EXPECT_EQ((signaled), __signaled); \
+		EXPECT_EQ((manual), __manual); \
+	})
+
 static int wait_objs(int fd, unsigned long request, __u32 count,
 		     const int *objs, __u32 owner, __u32 *index)
 {
@@ -353,6 +374,74 @@ TEST(mutex_state)
 	close(fd);
 }
 
+TEST(manual_event_state)
+{
+	struct ntsync_event_args event_args;
+	__u32 index, signaled;
+	int fd, event, ret;
+
+	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+	ASSERT_LE(0, fd);
+
+	event_args.manual = 1;
+	event_args.signaled = 0;
+	event_args.event = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, event_args.event);
+	event = event_args.event;
+	check_event_state(event, 0, 1);
+
+	signaled = 0xdeadbeef;
+	ret = ioctl(event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event, 1, 1);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+	check_event_state(event, 1, 1);
+
+	ret = wait_any(fd, 1, &event, 123, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+	check_event_state(event, 1, 1);
+
+	signaled = 0xdeadbeef;
+	ret = ioctl(event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+	check_event_state(event, 0, 1);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event, 0, 1);
+
+	ret = wait_any(fd, 1, &event, 123, &index);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(ETIMEDOUT, errno);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+	check_event_state(event, 0, 1);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event, 0, 1);
+
+	close(event);
+
+	close(fd);
+}
+
 TEST(test_wait_any)
 {
 	int objs[NTSYNC_MAX_WAIT_COUNT + 1], fd, ret;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 25/31] selftests: ntsync: Add some tests for auto-reset event state.
From: Elizabeth Figura @ 2024-02-14 23:53 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Test event-specific ioctls NTSYNC_IOC_EVENT_SET, NTSYNC_IOC_EVENT_RESET,
NTSYNC_IOC_EVENT_PULSE, NTSYNC_IOC_EVENT_READ for auto-reset events, and
waiting on auto-reset events.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 .../testing/selftests/drivers/ntsync/ntsync.c | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 7cc6fdf10abd..49ee4be280f6 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -442,6 +442,65 @@ TEST(manual_event_state)
 	close(fd);
 }
 
+TEST(auto_event_state)
+{
+	struct ntsync_event_args event_args;
+	__u32 index, signaled;
+	int fd, event, ret;
+
+	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+	ASSERT_LE(0, fd);
+
+	event_args.manual = 0;
+	event_args.signaled = 1;
+	event_args.event = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, event_args.event);
+	event = event_args.event;
+
+	check_event_state(event, 1, 0);
+
+	signaled = 0xdeadbeef;
+	ret = ioctl(event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+	check_event_state(event, 1, 0);
+
+	ret = wait_any(fd, 1, &event, 123, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+	check_event_state(event, 0, 0);
+
+	signaled = 0xdeadbeef;
+	ret = ioctl(event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event, 0, 0);
+
+	ret = wait_any(fd, 1, &event, 123, &index);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(ETIMEDOUT, errno);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+	check_event_state(event, 0, 0);
+
+	ret = ioctl(event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event, 0, 0);
+
+	close(event);
+
+	close(fd);
+}
+
 TEST(test_wait_any)
 {
 	int objs[NTSYNC_MAX_WAIT_COUNT + 1], fd, ret;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 26/31] selftests: ntsync: Add some tests for wakeup signaling with events.
From: Elizabeth Figura @ 2024-02-14 23:53 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Expand the contended wait tests, which previously only covered events and
semaphores, to cover events as well.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 .../testing/selftests/drivers/ntsync/ntsync.c | 151 +++++++++++++++++-
 1 file changed, 147 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 49ee4be280f6..bcf33bcf8c51 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -622,6 +622,7 @@ TEST(test_wait_any)
 
 TEST(test_wait_all)
 {
+	struct ntsync_event_args event_args = {0};
 	struct ntsync_mutex_args mutex_args = {0};
 	struct ntsync_sem_args sem_args = {0};
 	__u32 owner, index, count;
@@ -644,6 +645,11 @@ TEST(test_wait_all)
 	EXPECT_EQ(0, ret);
 	EXPECT_NE(0xdeadbeef, mutex_args.mutex);
 
+	event_args.manual = true;
+	event_args.signaled = true;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+
 	objs[0] = sem_args.sem;
 	objs[1] = mutex_args.mutex;
 
@@ -692,6 +698,14 @@ TEST(test_wait_all)
 	check_sem_state(sem_args.sem, 1, 3);
 	check_mutex_state(mutex_args.mutex, 1, 123);
 
+	objs[0] = sem_args.sem;
+	objs[1] = event_args.event;
+	ret = wait_all(fd, 2, objs, 123, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+	check_sem_state(sem_args.sem, 0, 3);
+	check_event_state(event_args.event, 1, 1);
+
 	/* test waiting on the same object twice */
 	objs[0] = objs[1] = sem_args.sem;
 	ret = wait_all(fd, 2, objs, 123, &index);
@@ -700,6 +714,7 @@ TEST(test_wait_all)
 
 	close(sem_args.sem);
 	close(mutex_args.mutex);
+	close(event_args.event);
 
 	close(fd);
 }
@@ -746,12 +761,13 @@ static int wait_for_thread(pthread_t thread, unsigned int ms)
 
 TEST(wake_any)
 {
+	struct ntsync_event_args event_args = {0};
 	struct ntsync_mutex_args mutex_args = {0};
 	struct ntsync_wait_args wait_args = {0};
 	struct ntsync_sem_args sem_args = {0};
 	struct wait_args thread_args;
+	__u32 count, index, signaled;
 	int objs[2], fd, ret;
-	__u32 count, index;
 	pthread_t thread;
 
 	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
@@ -833,10 +849,101 @@ TEST(wake_any)
 	EXPECT_EQ(0, thread_args.ret);
 	EXPECT_EQ(1, wait_args.index);
 
+	/* test waking events */
+
+	event_args.manual = false;
+	event_args.signaled = false;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+
+	objs[1] = event_args.event;
+	wait_args.timeout = get_abs_timeout(1000);
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event_args.event, 0, 0);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, thread_args.ret);
+	EXPECT_EQ(1, wait_args.index);
+
+	wait_args.timeout = get_abs_timeout(1000);
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event_args.event, 0, 0);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, thread_args.ret);
+	EXPECT_EQ(1, wait_args.index);
+
+	close(event_args.event);
+
+	event_args.manual = true;
+	event_args.signaled = false;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+
+	objs[1] = event_args.event;
+	wait_args.timeout = get_abs_timeout(1000);
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event_args.event, 1, 1);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, thread_args.ret);
+	EXPECT_EQ(1, wait_args.index);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+
+	wait_args.timeout = get_abs_timeout(1000);
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+	check_event_state(event_args.event, 0, 1);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, thread_args.ret);
+	EXPECT_EQ(1, wait_args.index);
+
+	close(event_args.event);
+
 	/* delete an object while it's being waited on */
 
 	wait_args.timeout = get_abs_timeout(200);
 	wait_args.owner = 123;
+	objs[1] = mutex_args.mutex;
 	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
 	EXPECT_EQ(0, ret);
 
@@ -856,12 +963,14 @@ TEST(wake_any)
 
 TEST(wake_all)
 {
+	struct ntsync_event_args manual_event_args = {0};
+	struct ntsync_event_args auto_event_args = {0};
 	struct ntsync_mutex_args mutex_args = {0};
 	struct ntsync_wait_args wait_args = {0};
 	struct ntsync_sem_args sem_args = {0};
 	struct wait_args thread_args;
-	int objs[2], fd, ret;
-	__u32 count, index;
+	__u32 count, index, signaled;
+	int objs[4], fd, ret;
 	pthread_t thread;
 
 	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
@@ -881,12 +990,24 @@ TEST(wake_all)
 	EXPECT_EQ(0, ret);
 	EXPECT_NE(0xdeadbeef, mutex_args.mutex);
 
+	manual_event_args.manual = true;
+	manual_event_args.signaled = true;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &manual_event_args);
+	EXPECT_EQ(0, ret);
+
+	auto_event_args.manual = false;
+	auto_event_args.signaled = true;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &auto_event_args);
+	EXPECT_EQ(0, ret);
+
 	objs[0] = sem_args.sem;
 	objs[1] = mutex_args.mutex;
+	objs[2] = manual_event_args.event;
+	objs[3] = auto_event_args.event;
 
 	wait_args.timeout = get_abs_timeout(1000);
 	wait_args.objs = (uintptr_t)objs;
-	wait_args.count = 2;
+	wait_args.count = 4;
 	wait_args.owner = 456;
 	thread_args.fd = fd;
 	thread_args.args = &wait_args;
@@ -920,12 +1041,32 @@ TEST(wake_all)
 
 	check_mutex_state(mutex_args.mutex, 0, 0);
 
+	ret = ioctl(manual_event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+
 	count = 2;
 	ret = post_sem(sem_args.sem, &count);
 	EXPECT_EQ(0, ret);
 	EXPECT_EQ(0, count);
+	check_sem_state(sem_args.sem, 2, 3);
+
+	ret = ioctl(auto_event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, signaled);
+
+	ret = ioctl(manual_event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+
+	ret = ioctl(auto_event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, signaled);
+
 	check_sem_state(sem_args.sem, 1, 3);
 	check_mutex_state(mutex_args.mutex, 1, 456);
+	check_event_state(manual_event_args.event, 1, 1);
+	check_event_state(auto_event_args.event, 0, 0);
 
 	ret = wait_for_thread(thread, 100);
 	EXPECT_EQ(0, ret);
@@ -943,6 +1084,8 @@ TEST(wake_all)
 
 	close(sem_args.sem);
 	close(mutex_args.mutex);
+	close(manual_event_args.event);
+	close(auto_event_args.event);
 
 	ret = wait_for_thread(thread, 200);
 	EXPECT_EQ(0, ret);
-- 
2.43.0


^ permalink raw reply related

* [PATCH 27/31] selftests: ntsync: Add tests for alertable waits.
From: Elizabeth Figura @ 2024-02-14 23:53 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Test the "alert" functionality of NTSYNC_IOC_WAIT_ALL and NTSYNC_IOC_WAIT_ANY,
when a wait is woken with an alert and when it is woken by an object.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 .../testing/selftests/drivers/ntsync/ntsync.c | 179 +++++++++++++++++-
 1 file changed, 176 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index bcf33bcf8c51..6c13b1f25d38 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -95,7 +95,7 @@ static int read_event_state(int event, __u32 *signaled, __u32 *manual)
 	})
 
 static int wait_objs(int fd, unsigned long request, __u32 count,
-		     const int *objs, __u32 owner, __u32 *index)
+		     const int *objs, __u32 owner, int alert, __u32 *index)
 {
 	struct ntsync_wait_args args = {0};
 	struct timespec timeout;
@@ -108,6 +108,7 @@ static int wait_objs(int fd, unsigned long request, __u32 count,
 	args.objs = (uintptr_t)objs;
 	args.owner = owner;
 	args.index = 0xdeadbeef;
+	args.alert = alert;
 	ret = ioctl(fd, request, &args);
 	*index = args.index;
 	return ret;
@@ -115,12 +116,26 @@ static int wait_objs(int fd, unsigned long request, __u32 count,
 
 static int wait_any(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
 {
-	return wait_objs(fd, NTSYNC_IOC_WAIT_ANY, count, objs, owner, index);
+	return wait_objs(fd, NTSYNC_IOC_WAIT_ANY, count, objs, owner, 0, index);
 }
 
 static int wait_all(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
 {
-	return wait_objs(fd, NTSYNC_IOC_WAIT_ALL, count, objs, owner, index);
+	return wait_objs(fd, NTSYNC_IOC_WAIT_ALL, count, objs, owner, 0, index);
+}
+
+static int wait_any_alert(int fd, __u32 count, const int *objs,
+			  __u32 owner, int alert, __u32 *index)
+{
+	return wait_objs(fd, NTSYNC_IOC_WAIT_ANY,
+			 count, objs, owner, alert, index);
+}
+
+static int wait_all_alert(int fd, __u32 count, const int *objs,
+			  __u32 owner, int alert, __u32 *index)
+{
+	return wait_objs(fd, NTSYNC_IOC_WAIT_ALL,
+			 count, objs, owner, alert, index);
 }
 
 TEST(semaphore_state)
@@ -1095,4 +1110,162 @@ TEST(wake_all)
 	close(fd);
 }
 
+TEST(alert_any)
+{
+	struct ntsync_event_args event_args = {0};
+	struct ntsync_sem_args sem_args = {0};
+	__u32 index, count, signaled;
+	int objs[2], fd, ret;
+
+	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+	ASSERT_LE(0, fd);
+
+	sem_args.count = 0;
+	sem_args.max = 2;
+	sem_args.sem = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, sem_args.sem);
+	objs[0] = sem_args.sem;
+
+	sem_args.count = 1;
+	sem_args.max = 2;
+	sem_args.sem = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, sem_args.sem);
+	objs[1] = sem_args.sem;
+
+	event_args.manual = true;
+	event_args.signaled = true;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_any_alert(fd, 0, NULL, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_any_alert(fd, 0, NULL, 123, event_args.event, &index);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(ETIMEDOUT, errno);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(1, index);
+
+	ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(2, index);
+
+	close(event_args.event);
+
+	/* test with an auto-reset event */
+
+	event_args.manual = false;
+	event_args.signaled = true;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+
+	count = 1;
+	ret = post_sem(objs[0], &count);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+
+	ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(2, index);
+
+	ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(ETIMEDOUT, errno);
+
+	close(event_args.event);
+
+	close(objs[0]);
+	close(objs[1]);
+
+	close(fd);
+}
+
+TEST(alert_all)
+{
+	struct ntsync_event_args event_args = {0};
+	struct ntsync_sem_args sem_args = {0};
+	__u32 index, count, signaled;
+	int objs[2], fd, ret;
+
+	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+	ASSERT_LE(0, fd);
+
+	sem_args.count = 2;
+	sem_args.max = 2;
+	sem_args.sem = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, sem_args.sem);
+	objs[0] = sem_args.sem;
+
+	sem_args.count = 1;
+	sem_args.max = 2;
+	sem_args.sem = 0xdeadbeef;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+	EXPECT_EQ(0, ret);
+	EXPECT_NE(0xdeadbeef, sem_args.sem);
+	objs[1] = sem_args.sem;
+
+	event_args.manual = true;
+	event_args.signaled = true;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+
+	ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(2, index);
+
+	close(event_args.event);
+
+	/* test with an auto-reset event */
+
+	event_args.manual = false;
+	event_args.signaled = true;
+	ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+
+	count = 2;
+	ret = post_sem(objs[1], &count);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, index);
+
+	ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(2, index);
+
+	ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+	EXPECT_EQ(-1, ret);
+	EXPECT_EQ(ETIMEDOUT, errno);
+
+	close(event_args.event);
+
+	close(objs[0]);
+	close(objs[1]);
+
+	close(fd);
+}
+
 TEST_HARNESS_MAIN
-- 
2.43.0


^ permalink raw reply related

* [PATCH 28/31] selftests: ntsync: Add some tests for wakeup signaling via alerts.
From: Elizabeth Figura @ 2024-02-14 23:53 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Expand the alert tests to cover alerting a thread mid-wait, to test that the
relevant scheduling logic works correctly.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 .../testing/selftests/drivers/ntsync/ntsync.c | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 6c13b1f25d38..40cfdd356ef4 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -1113,9 +1113,12 @@ TEST(wake_all)
 TEST(alert_any)
 {
 	struct ntsync_event_args event_args = {0};
+	struct ntsync_wait_args wait_args = {0};
 	struct ntsync_sem_args sem_args = {0};
 	__u32 index, count, signaled;
+	struct wait_args thread_args;
 	int objs[2], fd, ret;
+	pthread_t thread;
 
 	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
 	ASSERT_LE(0, fd);
@@ -1163,6 +1166,34 @@ TEST(alert_any)
 	EXPECT_EQ(0, ret);
 	EXPECT_EQ(2, index);
 
+	/* test wakeup via alert */
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+
+	wait_args.timeout = get_abs_timeout(1000);
+	wait_args.objs = (uintptr_t)objs;
+	wait_args.count = 2;
+	wait_args.owner = 123;
+	wait_args.index = 0xdeadbeef;
+	wait_args.alert = event_args.event;
+	thread_args.fd = fd;
+	thread_args.args = &wait_args;
+	thread_args.request = NTSYNC_IOC_WAIT_ANY;
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, thread_args.ret);
+	EXPECT_EQ(2, wait_args.index);
+
 	close(event_args.event);
 
 	/* test with an auto-reset event */
@@ -1199,9 +1230,12 @@ TEST(alert_any)
 TEST(alert_all)
 {
 	struct ntsync_event_args event_args = {0};
+	struct ntsync_wait_args wait_args = {0};
 	struct ntsync_sem_args sem_args = {0};
+	struct wait_args thread_args;
 	__u32 index, count, signaled;
 	int objs[2], fd, ret;
+	pthread_t thread;
 
 	fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
 	ASSERT_LE(0, fd);
@@ -1235,6 +1269,34 @@ TEST(alert_all)
 	EXPECT_EQ(0, ret);
 	EXPECT_EQ(2, index);
 
+	/* test wakeup via alert */
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+	EXPECT_EQ(0, ret);
+
+	wait_args.timeout = get_abs_timeout(1000);
+	wait_args.objs = (uintptr_t)objs;
+	wait_args.count = 2;
+	wait_args.owner = 123;
+	wait_args.index = 0xdeadbeef;
+	wait_args.alert = event_args.event;
+	thread_args.fd = fd;
+	thread_args.args = &wait_args;
+	thread_args.request = NTSYNC_IOC_WAIT_ALL;
+	ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(ETIMEDOUT, ret);
+
+	ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+
+	ret = wait_for_thread(thread, 100);
+	EXPECT_EQ(0, ret);
+	EXPECT_EQ(0, thread_args.ret);
+	EXPECT_EQ(2, wait_args.index);
+
 	close(event_args.event);
 
 	/* test with an auto-reset event */
-- 
2.43.0


^ permalink raw reply related

* [PATCH 29/31] selftests: ntsync: Add a stress test for contended waits.
From: Elizabeth Figura @ 2024-02-14 23:53 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Test a more realistic usage pattern, and one with heavy contention, in order to
actually exercise ntsync's internal synchronization.

This test has several threads in a tight loop acquiring a mutex, modifying some
shared data, and then releasing the mutex. At the end we check if the data is
consistent.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 .../testing/selftests/drivers/ntsync/ntsync.c | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 40cfdd356ef4..aba513c9af01 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -1330,4 +1330,78 @@ TEST(alert_all)
 	close(fd);
 }
 
+#define STRESS_LOOPS 10000
+#define STRESS_THREADS 4
+
+static unsigned int stress_counter;
+static int stress_device, stress_start_event, stress_mutex;
+
+static void *stress_thread(void *arg)
+{
+	struct ntsync_wait_args wait_args = {0};
+	__u32 index, count, i;
+	int ret;
+
+	wait_args.timeout = UINT64_MAX;
+	wait_args.count = 1;
+	wait_args.objs = (uintptr_t)&stress_start_event;
+	wait_args.owner = gettid();
+	wait_args.index = 0xdeadbeef;
+
+	ioctl(stress_device, NTSYNC_IOC_WAIT_ANY, &wait_args);
+
+	wait_args.objs = (uintptr_t)&stress_mutex;
+
+	for (i = 0; i < STRESS_LOOPS; ++i) {
+		ioctl(stress_device, NTSYNC_IOC_WAIT_ANY, &wait_args);
+
+		++stress_counter;
+
+		unlock_mutex(stress_mutex, wait_args.owner, &count);
+	}
+
+	return NULL;
+}
+
+TEST(stress_wait)
+{
+	struct ntsync_event_args event_args;
+	struct ntsync_mutex_args mutex_args;
+	pthread_t threads[STRESS_THREADS];
+	__u32 signaled, i;
+	int ret;
+
+	stress_device = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+	ASSERT_LE(0, stress_device);
+
+	mutex_args.owner = 0;
+	mutex_args.count = 0;
+	ret = ioctl(stress_device, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+	EXPECT_EQ(0, ret);
+	stress_mutex = mutex_args.mutex;
+
+	event_args.manual = 1;
+	event_args.signaled = 0;
+	ret = ioctl(stress_device, NTSYNC_IOC_CREATE_EVENT, &event_args);
+	EXPECT_EQ(0, ret);
+	stress_start_event = event_args.event;
+
+	for (i = 0; i < STRESS_THREADS; ++i)
+		pthread_create(&threads[i], NULL, stress_thread, NULL);
+
+	ret = ioctl(stress_start_event, NTSYNC_IOC_EVENT_SET, &signaled);
+	EXPECT_EQ(0, ret);
+
+	for (i = 0; i < STRESS_THREADS; ++i) {
+		ret = pthread_join(threads[i], NULL);
+		EXPECT_EQ(0, ret);
+	}
+
+	EXPECT_EQ(STRESS_LOOPS * STRESS_THREADS, stress_counter);
+
+	close(stress_start_event);
+	close(stress_mutex);
+	close(stress_device);
+}
+
 TEST_HARNESS_MAIN
-- 
2.43.0


^ permalink raw reply related

* [PATCH 30/31] maintainers: Add an entry for ntsync.
From: Elizabeth Figura @ 2024-02-14 23:53 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Add myself as maintainer, supported by CodeWeavers.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 960512bec428..e80ed255951b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15591,6 +15591,15 @@ T:	git https://github.com/Paragon-Software-Group/linux-ntfs3.git
 F:	Documentation/filesystems/ntfs3.rst
 F:	fs/ntfs3/
 
+NTSYNC SYNCHRONIZATION PRIMITIVE DRIVER
+M:	Elizabeth Figura <zfigura@codeweavers.com>
+L:	wine-devel@winehq.org
+S:	Supported
+F:	Documentation/userspace-api/ntsync.rst
+F:	drivers/misc/ntsync.c
+F:	include/uapi/linux/ntsync.h
+F:	tools/testing/selftests/drivers/ntsync/
+
 NUBUS SUBSYSTEM
 M:	Finn Thain <fthain@linux-m68k.org>
 L:	linux-m68k@lists.linux-m68k.org
-- 
2.43.0


^ permalink raw reply related

* [PATCH 31/31] docs: ntsync: Add documentation for the ntsync uAPI.
From: Elizabeth Figura @ 2024-02-14 23:53 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240214235307.10494-1-zfigura@codeweavers.com>

Add an overall explanation of the driver architecture, and complete and precise
specification for its intended behaviour.

Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
 Documentation/userspace-api/index.rst  |   1 +
 Documentation/userspace-api/ntsync.rst | 399 +++++++++++++++++++++++++
 2 files changed, 400 insertions(+)
 create mode 100644 Documentation/userspace-api/ntsync.rst

diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index 09f61bd2ac2e..f5a72ed27def 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -34,6 +34,7 @@ place where this information is gathered.
    tee
    isapnp
    dcdbas
+   ntsync
 
 .. only::  subproject and html
 
diff --git a/Documentation/userspace-api/ntsync.rst b/Documentation/userspace-api/ntsync.rst
new file mode 100644
index 000000000000..202c2350d3af
--- /dev/null
+++ b/Documentation/userspace-api/ntsync.rst
@@ -0,0 +1,399 @@
+===================================
+NT synchronization primitive driver
+===================================
+
+This page documents the user-space API for the ntsync driver.
+
+ntsync is a support driver for emulation of NT synchronization
+primitives by user-space NT emulators. It exists because implementation
+in user-space, using existing tools, cannot match Windows performance
+while offering accurate semantics. It is implemented entirely in
+software, and does not drive any hardware device.
+
+This interface is meant as a compatibility tool only, and should not
+be used for general synchronization. Instead use generic, versatile
+interfaces such as futex(2) and poll(2).
+
+Synchronization primitives
+==========================
+
+The ntsync driver exposes three types of synchronization primitives:
+semaphores, mutexes, and events.
+
+A semaphore holds a single volatile 32-bit counter, and a static 32-bit
+integer denoting the maximum value. It is considered signaled when the
+counter is nonzero. The counter is decremented by one when a wait is
+satisfied. Both the initial and maximum count are established when the
+semaphore is created.
+
+A mutex holds a volatile 32-bit recursion count, and a volatile 32-bit
+identifier denoting its owner. A mutex is considered signaled when its
+owner is zero (indicating that it is not owned). The recursion count is
+incremented when a wait is satisfied, and ownership is set to the given
+identifier.
+
+A mutex also holds an internal flag denoting whether its previous owner
+has died; such a mutex is said to be abandoned. Owner death is not
+tracked automatically based on thread death, but rather must be
+communicated using ``NTSYNC_IOC_MUTEX_KILL``. An abandoned mutex is
+inherently considered unowned.
+
+Except for the "unowned" semantics of zero, the actual value of the
+owner identifier is not interpreted by the ntsync driver at all. The
+intended use is to store a thread identifier; however, the ntsync
+driver does not actually validate that a calling thread provides
+consistent or unique identifiers.
+
+An event holds a volatile boolean state denoting whether it is signaled
+or not. There are two types of events, auto-reset and manual-reset. An
+auto-reset event is designaled when a wait is satisfied; a manual-reset
+event is not. The event type is specified when the event is created.
+
+Unless specified otherwise, all operations on an object are atomic and
+totally ordered with respect to other operations on the same object.
+
+Objects are represented by files. When all file descriptors to an
+object are closed, that object is deleted.
+
+Char device
+===========
+
+The ntsync driver creates a single char device /dev/ntsync. Each file
+description opened on the device represents a unique instance intended
+to back an individual NT virtual machine. Objects created by one ntsync
+instance may only be used with other objects created by the same
+instance.
+
+ioctl reference
+===============
+
+All operations on the device are done through ioctls. There are four
+structures used in ioctl calls::
+
+   struct ntsync_sem_args {
+   	__u32 sem;
+   	__u32 count;
+   	__u32 max;
+   };
+
+   struct ntsync_mutex_args {
+   	__u32 mutex;
+   	__u32 owner;
+   	__u32 count;
+   };
+
+   struct ntsync_event_args {
+   	__u32 event;
+   	__u32 signaled;
+   	__u32 manual;
+   };
+
+   struct ntsync_wait_args {
+   	__u64 timeout;
+   	__u64 objs;
+   	__u32 count;
+   	__u32 owner;
+   	__u32 index;
+   	__u32 alert;
+   	__u32 flags;
+   	__u32 pad;
+   };
+
+Depending on the ioctl, members of the structure may be used as input,
+output, or not at all. All ioctls return 0 on success.
+
+The ioctls on the device file are as follows:
+
+.. c:macro:: NTSYNC_IOC_CREATE_SEM
+
+  Create a semaphore object. Takes a pointer to struct
+  :c:type:`ntsync_sem_args`, which is used as follows:
+
+  .. list-table::
+
+     * - ``sem``
+       - On output, contains a file descriptor to the created semaphore.
+     * - ``count``
+       - Initial count of the semaphore.
+     * - ``max``
+       - Maximum count of the semaphore.
+
+  Fails with ``EINVAL`` if ``count`` is greater than ``max``.
+
+.. c:macro:: NTSYNC_IOC_CREATE_MUTEX
+
+  Create a mutex object. Takes a pointer to struct
+  :c:type:`ntsync_mutex_args`, which is used as follows:
+
+  .. list-table::
+
+     * - ``mutex``
+       - On output, contains a file descriptor to the created mutex.
+     * - ``count``
+       - Initial recursion count of the mutex.
+     * - ``owner``
+       - Initial owner of the mutex.
+
+  If ``owner`` is nonzero and ``count`` is zero, or if ``owner`` is
+  zero and ``count`` is nonzero, the function fails with ``EINVAL``.
+
+.. c:macro:: NTSYNC_IOC_CREATE_EVENT
+
+  Create an event object. Takes a pointer to struct
+  :c:type:`ntsync_event_args`, which is used as follows:
+
+  .. list-table::
+
+     * - ``event``
+       - On output, contains a file descriptor to the created event.
+     * - ``signaled``
+       - If nonzero, the event is initially signaled, otherwise
+         nonsignaled.
+     * - ``manual``
+       - If nonzero, the event is a manual-reset event, otherwise
+         auto-reset.
+
+The ioctls on the individual objects are as follows:
+
+.. c:macro:: NTSYNC_IOC_SEM_POST
+
+  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
+  output contains its previous count.
+
+  If adding to the semaphore's current count would raise the latter
+  past the semaphore's maximum count, the ioctl fails with
+  ``EOVERFLOW`` and the semaphore is not affected. If raising the
+  semaphore's count causes it to become signaled, eligible threads
+  waiting on this semaphore will be woken and the semaphore's count
+  decremented appropriately.
+
+.. c:macro:: NTSYNC_IOC_MUTEX_UNLOCK
+
+  Release a mutex object. Takes a pointer to struct
+  :c:type:`ntsync_mutex_args`, which is used as follows:
+
+  .. list-table::
+
+     * - ``mutex``
+       - Ignored.
+     * - ``owner``
+       - Specifies the owner trying to release this mutex.
+     * - ``count``
+       - On output, contains the previous recursion count.
+
+  If ``owner`` is zero, the ioctl fails with ``EINVAL``. If ``owner``
+  is not the current owner of the mutex, the ioctl fails with
+  ``EPERM``.
+
+  The mutex's count will be decremented by one. If decrementing the
+  mutex's count causes it to become zero, the mutex is marked as
+  unowned and signaled, and eligible threads waiting on it will be
+  woken as appropriate.
+
+.. c:macro:: NTSYNC_IOC_SET_EVENT
+
+  Signal an event object. Takes a pointer to a 32-bit integer, which on
+  output contains the previous state of the event.
+
+  Eligible threads will be woken, and auto-reset events will be
+  designaled appropriately.
+
+.. c:macro:: NTSYNC_IOC_RESET_EVENT
+
+  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
+
+  Wake threads waiting on an event object while leaving it in an
+  unsignaled state. Takes a pointer to a 32-bit integer, which on
+  output contains the previous state of the event.
+
+  A pulse operation can be thought of as a set followed by a reset,
+  performed as a single atomic operation. If two threads are waiting on
+  an auto-reset event which is pulsed, only one will be woken. If two
+  threads are waiting a manual-reset event which is pulsed, both will
+  be woken. However, in both cases, the event will be unsignaled
+  afterwards, and a simultaneous read operation will always report the
+  event as unsignaled.
+
+.. c:macro:: NTSYNC_IOC_READ_SEM
+
+  Read the current state of a semaphore object. Takes a pointer to
+  struct :c:type:`ntsync_sem_args`, which is used as follows:
+
+  .. list-table::
+
+     * - ``sem``
+       - Ignored.
+     * - ``count``
+       - On output, contains the current count of the semaphore.
+     * - ``max``
+       - On output, contains the maximum count of the semaphore.
+
+.. c:macro:: NTSYNC_IOC_READ_MUTEX
+
+  Read the current state of a mutex object. Takes a pointer to struct
+  :c:type:`ntsync_mutex_args`, which is used as follows:
+
+  .. list-table::
+
+     * - ``mutex``
+       - Ignored.
+     * - ``owner``
+       - On output, contains the current owner of the mutex, or zero
+         if the mutex is not currently owned.
+     * - ``count``
+       - On output, contains the current recursion count of the mutex.
+
+  If the mutex is marked as abandoned, the function fails with
+  ``EOWNERDEAD``. In this case, ``count`` and ``owner`` are set to
+  zero.
+
+.. c:macro:: NTSYNC_IOC_READ_EVENT
+
+  Read the current state of an event object. Takes a pointer to struct
+  :c:type:`ntsync_event_args`, which is used as follows:
+
+  .. list-table::
+
+     * - ``event``
+       - Ignored.
+     * - ``signaled``
+       - On output, contains the current state of the event.
+     * - ``manual``
+       - On output, contains 1 if the event is a manual-reset event,
+         and 0 otherwise.
+
+.. c:macro:: NTSYNC_IOC_KILL_OWNER
+
+  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
+  owner. If the owner is zero, the ioctl fails with ``EINVAL``. If the
+  owner does not own the mutex, the function fails with ``EPERM``.
+
+  Eligible threads waiting on the mutex will be woken as appropriate
+  (and such waits will fail with ``EOWNERDEAD``, as described below).
+
+.. c:macro:: NTSYNC_IOC_WAIT_ANY
+
+  Poll on any of a list of objects, atomically acquiring at most one.
+  Takes a pointer to struct :c:type:`ntsync_wait_args`, which is
+  used as follows:
+
+  .. list-table::
+
+     * - ``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``.
+     * - ``objs``
+       - Pointer to an array of ``count`` file descriptors
+         (specified as an integer so that the structure has the same
+         size regardless of architecture). If any object is
+         invalid, the function fails with ``EINVAL``.
+     * - ``count``
+       - Number of objects specified in the ``objs`` array.
+         If greater than ``NTSYNC_MAX_WAIT_COUNT``, the function fails
+         with ``EINVAL``.
+     * - ``owner``
+       - Mutex owner identifier. If any object in ``objs`` is a mutex,
+         the ioctl will attempt to acquire that mutex on behalf of
+         ``owner``. If ``owner`` is zero, the ioctl fails with
+         ``EINVAL``.
+     * - ``index``
+       - On success, contains the index (into ``objs``) of the object
+         which was signaled. If ``alert`` was signaled instead,
+         this contains ``count``.
+     * - ``alert``
+       - Optional event object file descriptor. If nonzero, this
+         specifies an "alert" event object which, if signaled, will
+         terminate the wait. If nonzero, the identifier must point to a
+         valid event.
+     * - ``flags``
+       - Zero or more flags. Currently the only flag is
+         ``NTSYNC_WAIT_REALTIME``, which causes the timeout to be
+         measured against the REALTIME clock instead of MONOTONIC.
+     * - ``pad``
+       - Unused, must be set to zero.
+
+  This function attempts to acquire one of the given objects. If unable
+  to do so, it sleeps until an object becomes signaled, subsequently
+  acquiring it, or the timeout expires. In the latter case the ioctl
+  fails with ``ETIMEDOUT``. The function only acquires one object, even
+  if multiple objects are signaled.
+
+  A semaphore is considered to be signaled if its count is nonzero, and
+  is acquired by decrementing its count by one. A mutex is considered
+  to be signaled if it is unowned or if its owner matches the ``owner``
+  argument, and is acquired by incrementing its recursion count by one
+  and setting its owner to the ``owner`` argument. An auto-reset event
+  is acquired by designaling it; a manual-reset event is not affected
+  by acquisition.
+
+  Acquisition is atomic and totally ordered with respect to other
+  operations on the same object. If two wait operations (with different
+  ``owner`` identifiers) are queued on the same mutex, only one is
+  signaled. If two wait operations are queued on the same semaphore,
+  and a value of one is posted to it, only one is signaled. The order
+  in which threads are signaled is not specified.
+
+  If an abandoned mutex is acquired, the ioctl fails with
+  ``EOWNERDEAD``. Although this is a failure return, the function may
+  otherwise be considered successful. The mutex is marked as owned by
+  the given owner (with a recursion count of 1) and as no longer
+  abandoned, and ``index`` is still set to the index of the mutex.
+
+  The ``alert`` argument is an "extra" event which can terminate the
+  wait, independently of all other objects. If members of ``objs`` and
+  ``alert`` are both simultaneously signaled, a member of ``objs`` will
+  always be given priority and acquired first.
+
+  It is valid to pass the same object more than once, including by
+  passing the same event in the ``objs`` array and in ``alert``. If a
+  wakeup occurs due to that object being signaled, ``index`` is set to
+  the lowest index corresponding to that object.
+
+  The function may fail with ``EINTR`` if a signal is received.
+
+.. c:macro:: NTSYNC_IOC_WAIT_ALL
+
+  Poll on a list of objects, atomically acquiring all of them. Takes a
+  pointer to struct :c:type:`ntsync_wait_args`, which is used
+  identically to ``NTSYNC_IOC_WAIT_ANY``, except that ``index`` is
+  always filled with zero on success if not woken via alert.
+
+  This function attempts to simultaneously acquire all of the given
+  objects. If unable to do so, it sleeps until all objects become
+  simultaneously signaled, subsequently acquiring them, or the timeout
+  expires. In the latter case the ioctl fails with ``ETIMEDOUT`` and no
+  objects are modified.
+
+  Objects may become signaled and subsequently designaled (through
+  acquisition by other threads) while this thread is sleeping. Only
+  once all objects are simultaneously signaled does the ioctl acquire
+  them and return. The entire acquisition is atomic and totally ordered
+  with respect to other operations on any of the given objects.
+
+  If an abandoned mutex is acquired, the ioctl fails with
+  ``EOWNERDEAD``. Similarly to ``NTSYNC_IOC_WAIT_ANY``, all objects are
+  nevertheless marked as acquired. Note that if multiple mutex objects
+  are specified, there is no way to know which were marked as
+  abandoned.
+
+  As with "any" waits, the ``alert`` argument is an "extra" event which
+  can terminate the wait. Critically, however, an "all" wait will
+  succeed if all members in ``objs`` are signaled, *or* if ``alert`` is
+  signaled. In the latter case ``index`` will be set to ``count``. As
+  with "any" waits, if both conditions are filled, the former takes
+  priority, and objects in ``objs`` will be acquired.
+
+  Unlike ``NTSYNC_IOC_WAIT_ANY``, it is not valid to pass the same
+  object more than once, nor is it valid to pass the same object in
+  ``objs`` and in ``alert``. If this is attempted, the function fails
+  with ``EINVAL``.
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH 00/31] NT synchronization primitive driver
From: Elizabeth Figura @ 2024-02-15  0:01 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest
In-Reply-To: <20240214233645.9273-1-zfigura@codeweavers.com>

On Wednesday, 14 February 2024 17:36:36 CST Elizabeth Figura wrote:
> This patch series introduces a new char misc driver, /dev/ntsync, which is used
> to implement Windows NT synchronization primitives.

Ugh, sorry, I made a bit of a mess while sending this revision. I accidentally
sent 000* instead of 00*, and then tried to fix it by sending the rest with
--in-reply-to but forgot to also add --no-thread. Please let me know if I should
resend the whole series.

--Zeb



^ permalink raw reply

* Re: [PATCH 01/31] ntsync: Introduce the ntsync driver and character device.
From: Randy Dunlap @ 2024-02-15  1:57 UTC (permalink / raw)
  To: Elizabeth Figura, Arnd Bergmann, Greg Kroah-Hartman,
	Jonathan Corbet, Shuah Khan
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest
In-Reply-To: <20240214233645.9273-2-zfigura@codeweavers.com>

Hi,

On 2/14/24 15:36, Elizabeth Figura wrote:
> ntsync uses a misc device as the simplest and least intrusive uAPI interface.
> 
> Each file description on the device represents an isolated NT instance, intended
> to correspond to a single NT virtual machine.
> 
> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
> ---
>  drivers/misc/Kconfig  |  9 ++++++++
>  drivers/misc/Makefile |  1 +
>  drivers/misc/ntsync.c | 52 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 62 insertions(+)
>  create mode 100644 drivers/misc/ntsync.c
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 4fb291f0bf7c..bdd8a71bd853 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -504,6 +504,15 @@ config OPEN_DICE
>  	  measured boot flow. Userspace can use CDIs for remote attestation
>  	  and sealing.
>  
> +config NTSYNC
> +	tristate "NT synchronization primitive emulation"
> +	help
> +	  This module provides kernel support for emulation of Windows NT
> +	  synchronization primitives. It is not a hardware driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ntsync.
> +
>  	  If unsure, say N.

It looks like the "If unsure" line belongs to the OPEN_DICE kconfig entry
above here. If you want one for NTSYNC, please add one.

>  
>  config VCPU_STALL_DETECTOR


> diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
> new file mode 100644
> index 000000000000..e4969ef90722
> --- /dev/null
> +++ b/drivers/misc/ntsync.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ntsync.c - Kernel driver for NT synchronization primitives
> + *
> + * Copyright (C) 2024 Elizabeth Figura

It would be nice to have your email address above...

> + */
> +

> +
> +MODULE_AUTHOR("Elizabeth Figura");

but at least please add it in MODULE_AUTHOR(). Yes it's optional, but
roughly 7900 of 10400 entries do include it (according to my rough
grepping).

Yes, I know that it's in MAINTAINERS.

> +MODULE_DESCRIPTION("Kernel driver for NT synchronization primitives");
> +MODULE_LICENSE("GPL");

thanks.
-- 
#Randy

^ permalink raw reply

* Re: [PATCH RFT v5 1/7] Documentation: userspace-api: Add shadow stack API documentation
From: Deepak Gupta @ 2024-02-15  4:08 UTC (permalink / raw)
  To: Mark Brown
  Cc: Rick P. Edgecombe, Szabolcs Nagy, H.J. Lu, Florian Weimer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider, Christian Brauner,
	Shuah Khan, linux-kernel, Catalin Marinas, Will Deacon, Kees Cook,
	jannh, linux-kselftest, linux-api
In-Reply-To: <20240203-clone3-shadow-stack-v5-1-322c69598e4b@kernel.org>

On Fri, Feb 2, 2024 at 4:05 PM Mark Brown <broonie@kernel.org> wrote:
>
> There are a number of architectures with shadow stack features which we are
> presenting to userspace with as consistent an API as we can (though there
> are some architecture specifics). Especially given that there are some
> important considerations for userspace code interacting directly with the
> feature let's provide some documentation covering the common aspects.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  Documentation/userspace-api/index.rst        |  1 +
>  Documentation/userspace-api/shadow_stack.rst | 41 ++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
> index 09f61bd2ac2e..c142183d9c98 100644
> --- a/Documentation/userspace-api/index.rst
> +++ b/Documentation/userspace-api/index.rst
> @@ -27,6 +27,7 @@ place where this information is gathered.
>     iommufd
>     media/index
>     netlink/index
> +   shadow_stack
>     sysfs-platform_profile
>     vduse
>     futex2
> diff --git a/Documentation/userspace-api/shadow_stack.rst b/Documentation/userspace-api/shadow_stack.rst
> new file mode 100644
> index 000000000000..c6e5ab795b60
> --- /dev/null
> +++ b/Documentation/userspace-api/shadow_stack.rst
> @@ -0,0 +1,41 @@
> +=============
> +Shadow Stacks
> +=============
> +
> +Introduction
> +============
> +
> +Several architectures have features which provide backward edge
> +control flow protection through a hardware maintained stack, only
> +writeable by userspace through very limited operations.  This feature
> +is referred to as shadow stacks on Linux, on x86 it is part of Intel
> +Control Enforcement Technology (CET), on arm64 it is Guarded Control
> +Stacks feature (FEAT_GCS) and for RISC-V it is the Zicfiss extension.
> +It is expected that this feature will normally be managed by the
> +system dynamic linker and libc in ways broadly transparent to
> +application code, this document covers interfaces and considerations
> +
> +
> +Enabling
> +========
> +
> +Shadow stacks default to disabled when a userspace process is
> +executed, they can be enabled for the current thread with a syscall:
> +
> + - For x86 the ARCH_SHSTK_ENABLE arch_prctl()
> +
> +It is expected that this will normally be done by the dynamic linker.
> +Any new threads created by a thread with shadow stacks enabled will
> +themsleves have shadow stacks enabled.
> +
> +
> +Enablement considerations
> +=========================
> +
> +- Returning from the function that enables shadow stacks without first
> +  disabling them will cause a shadow stack exception.  This includes
> +  any syscall wrapper or other library functions, the syscall will need
> +  to be inlined.
> +- A lock feature allows userspace to prevent disabling of shadow stacks.
> +- This that change the stack context like longjmp() or use of ucontext
> +  changes on signal return will need support from libc.

For enabling considerations, it will be good to have some words on ss tokens
too, probably along the following lines.

- Shadow stack tokens: During shadow stack switches (either by user mode
or kernel), to prevent inadvertent shadow stack pivoting, it is expected to save
some predefined formatted token during shadow stack save operation and
validating the token during shadow stack restore operation.

>
> --
> 2.30.2
>

^ permalink raw reply

* Re: [PATCH RFT v5 3/7] mm: Introduce ARCH_HAS_USER_SHADOW_STACK
From: Deepak Gupta @ 2024-02-15  4:14 UTC (permalink / raw)
  To: Mark Brown
  Cc: Rick P. Edgecombe, Szabolcs Nagy, H.J. Lu, Florian Weimer,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider, Christian Brauner,
	Shuah Khan, linux-kernel, Catalin Marinas, Will Deacon, Kees Cook,
	jannh, linux-kselftest, linux-api, David Hildenbrand
In-Reply-To: <20240203-clone3-shadow-stack-v5-3-322c69598e4b@kernel.org>

On Fri, Feb 2, 2024 at 4:05 PM Mark Brown <broonie@kernel.org> wrote:
>
> Since multiple architectures have support for shadow stacks and we need to
> select support for this feature in several places in the generic code
> provide a generic config option that the architectures can select.
>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Deepak Gupta <debug@rivosinc.com

^ permalink raw reply

* Re: [PATCH 00/31] NT synchronization primitive driver
From: Greg Kroah-Hartman @ 2024-02-15  7:24 UTC (permalink / raw)
  To: Elizabeth Figura
  Cc: Arnd Bergmann, Jonathan Corbet, Shuah Khan, linux-kernel,
	linux-api, wine-devel, André Almeida, Wolfram Sang,
	Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski, linux-doc,
	linux-kselftest
In-Reply-To: <5754084.DvuYhMxLoT@camazotz>

On Wed, Feb 14, 2024 at 06:01:22PM -0600, Elizabeth Figura wrote:
> On Wednesday, 14 February 2024 17:36:36 CST Elizabeth Figura wrote:
> > This patch series introduces a new char misc driver, /dev/ntsync, which is used
> > to implement Windows NT synchronization primitives.
> 
> Ugh, sorry, I made a bit of a mess while sending this revision. I accidentally
> sent 000* instead of 00*, and then tried to fix it by sending the rest with
> --in-reply-to but forgot to also add --no-thread. Please let me know if I should
> resend the whole series.

Please resend the whole thing, as a v2 series, as our tools can't pick
this up easily as-is.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 02/31] ntsync: Introduce NTSYNC_IOC_CREATE_SEM.
From: Greg Kroah-Hartman @ 2024-02-15  7:28 UTC (permalink / raw)
  To: Elizabeth Figura
  Cc: Arnd Bergmann, Jonathan Corbet, Shuah Khan, linux-kernel,
	linux-api, wine-devel, André Almeida, Wolfram Sang,
	Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski, linux-doc,
	linux-kselftest
In-Reply-To: <20240214233645.9273-3-zfigura@codeweavers.com>

On Wed, Feb 14, 2024 at 05:36:38PM -0600, Elizabeth Figura wrote:
> This corresponds to the NT syscall NtCreateSemaphore().
> 
> Semaphores are one of three types of object to be implemented in this driver,
> the others being mutexes and events.
> 
> An NT semaphore contains a 32-bit counter, and is signaled and can be acquired
> when the counter is nonzero. The counter has a maximum value which is specified
> at creation time. The initial value of the semaphore is also specified at
> creation time. There are no restrictions on the maximum and initial value.
> 
> Each object is exposed as an file, to which any number of fds may be opened.
> When all fds are closed, the object is deleted.
> 
> Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
> ---
>  .../userspace-api/ioctl/ioctl-number.rst      |   2 +
>  drivers/misc/ntsync.c                         | 120 ++++++++++++++++++
>  include/uapi/linux/ntsync.h                   |  21 +++
>  3 files changed, 143 insertions(+)
>  create mode 100644 include/uapi/linux/ntsync.h
> 
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 457e16f06e04..2f5c6994f042 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -173,6 +173,8 @@ Code  Seq#    Include File                                           Comments
>  'M'   00-0F  drivers/video/fsl-diu-fb.h                              conflict!
>  'N'   00-1F  drivers/usb/scanner.h
>  'N'   40-7F  drivers/block/nvme.c
> +'N'   80-8F  uapi/linux/ntsync.h                                     NT synchronization primitives
> +                                                                     <mailto:wine-devel@winehq.org>
>  'O'   00-06  mtd/ubi-user.h                                          UBI
>  'P'   all    linux/soundcard.h                                       conflict!
>  'P'   60-6F  sound/sscape_ioctl.h                                    conflict!
> diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
> index e4969ef90722..3ad86d98b82d 100644
> --- a/drivers/misc/ntsync.c
> +++ b/drivers/misc/ntsync.c
> @@ -5,26 +5,146 @@
>   * Copyright (C) 2024 Elizabeth Figura
>   */
>  
> +#include <linux/anon_inodes.h>
> +#include <linux/file.h>
>  #include <linux/fs.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/ntsync.h>
>  
>  #define NTSYNC_NAME	"ntsync"
>  
> +enum ntsync_type {
> +	NTSYNC_TYPE_SEM,
> +};
> +
> +struct ntsync_obj {
> +	enum ntsync_type type;
> +
> +	union {
> +		struct {
> +			__u32 count;
> +			__u32 max;
> +		} sem;
> +	} u;
> +
> +	struct file *file;
> +	struct ntsync_device *dev;
> +};
> +
> +struct ntsync_device {
> +	struct file *file;
> +};

No reference counting is needed for your ntsync_device?  Or are you
relying on the reference counting of struct file here?

You pass around pointers to this structure, and save it off into other
structures, how do you know it is "safe" to do so?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v2] uapi/auxvec: Define AT_HWCAP3 and AT_HWCAP4 aux vector, entries
From: Arnd Bergmann @ 2024-02-15  8:16 UTC (permalink / raw)
  To: Peter Bergner, linux-api, Linux-Arch,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
  Cc: Adhemerval Zanella Netto, Szabolcs Nagy, Nick Piggin,
	Michael Ellerman
In-Reply-To: <a406b535-dc55-4856-8ae9-5a063644a1af@linux.ibm.com>

On Wed, Feb 14, 2024, at 23:34, Peter Bergner wrote:
> The powerpc toolchain keeps a copy of the HWCAP bit masks in our TCB for fast
> access by the __builtin_cpu_supports built-in function.  The TCB space for
> the HWCAP entries - which are created in pairs - is an ABI extension, so
> waiting to create the space for HWCAP3 and HWCAP4 until we need them is
> problematical.  Define AT_HWCAP3 and AT_HWCAP4 in the generic uapi header
> so they can be used in glibc to reserve space in the powerpc TCB for their
> future use.
>
> I scanned through the Linux and GLIBC source codes looking for unused AT_*
> values and 29 and 30 did not seem to be used, so they are what I went
> with.  This has received Acked-by's from both GLIBC and Linux kernel
> developers and no reservations or Nacks from anyone.
>
> Arnd, we seem to have consensus on the patch below.  Is this something
> you could take and apply to your tree? 
>

I don't mind taking it, but it may be better to use the
powerpc tree if that is where it's actually being used.

If Michael takes it, please add

Acked-by: Arnd Bergmann <arnd@arndb.de>

      Arnd

^ permalink raw reply

* Re: [PATCH v2] uapi/auxvec: Define AT_HWCAP3 and AT_HWCAP4 aux vector, entries
From: Peter Bergner @ 2024-02-15 14:39 UTC (permalink / raw)
  To: Arnd Bergmann, linux-api, Linux-Arch,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
  Cc: Adhemerval Zanella Netto, Szabolcs Nagy, Nick Piggin,
	Michael Ellerman
In-Reply-To: <aa657f01-7cb1-43f4-947e-173fc8a53f1f@app.fastmail.com>

On 2/15/24 2:16 AM, Arnd Bergmann wrote:
> On Wed, Feb 14, 2024, at 23:34, Peter Bergner wrote:
>> The powerpc toolchain keeps a copy of the HWCAP bit masks in our TCB for fast
>> access by the __builtin_cpu_supports built-in function.  The TCB space for
>> the HWCAP entries - which are created in pairs - is an ABI extension, so
>> waiting to create the space for HWCAP3 and HWCAP4 until we need them is
>> problematical.  Define AT_HWCAP3 and AT_HWCAP4 in the generic uapi header
>> so they can be used in glibc to reserve space in the powerpc TCB for their
>> future use.
>>
>> I scanned through the Linux and GLIBC source codes looking for unused AT_*
>> values and 29 and 30 did not seem to be used, so they are what I went
>> with.  This has received Acked-by's from both GLIBC and Linux kernel
>> developers and no reservations or Nacks from anyone.
>>
>> Arnd, we seem to have consensus on the patch below.  Is this something
>> you could take and apply to your tree? 
>>
> 
> I don't mind taking it, but it may be better to use the
> powerpc tree if that is where it's actually being used.

So this is not a powerpc only patch, but we may be the first arch
to use it.  Szabolcs mentioned that aarch64 was pretty quickly filling
up their AT_HWCAP2 and that they will eventually require using AT_HWCAP3
as well.  If you still think this should go through the powerpc tree,
I can check on that.

Peter



^ permalink raw reply

* Re: [PATCH 01/31] ntsync: Introduce the ntsync driver and character device.
From: Elizabeth Figura @ 2024-02-15 19:04 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan,
	Randy Dunlap
  Cc: linux-kernel, linux-api, wine-devel, André Almeida,
	Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	linux-doc, linux-kselftest
In-Reply-To: <51a442ec-3835-4282-982b-734c0962141c@infradead.org>

On Wednesday, 14 February 2024 19:57:23 CST Randy Dunlap wrote:
> Hi,
> 
> On 2/14/24 15:36, Elizabeth Figura wrote:
> > ntsync uses a misc device as the simplest and least intrusive uAPI interface.
> > 
> > Each file description on the device represents an isolated NT instance, intended
> > to correspond to a single NT virtual machine.
> > 
> > Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
> > ---
> >  drivers/misc/Kconfig  |  9 ++++++++
> >  drivers/misc/Makefile |  1 +
> >  drivers/misc/ntsync.c | 52 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 62 insertions(+)
> >  create mode 100644 drivers/misc/ntsync.c
> > 
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index 4fb291f0bf7c..bdd8a71bd853 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -504,6 +504,15 @@ config OPEN_DICE
> >  	  measured boot flow. Userspace can use CDIs for remote attestation
> >  	  and sealing.
> >  
> > +config NTSYNC
> > +	tristate "NT synchronization primitive emulation"
> > +	help
> > +	  This module provides kernel support for emulation of Windows NT
> > +	  synchronization primitives. It is not a hardware driver.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called ntsync.
> > +
> >  	  If unsure, say N.
> 
> It looks like the "If unsure" line belongs to the OPEN_DICE kconfig entry
> above here. If you want one for NTSYNC, please add one.

Oops, looks like that was a bad rebase :-(

> 
> >  
> >  config VCPU_STALL_DETECTOR
> 
> 
> > diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
> > new file mode 100644
> > index 000000000000..e4969ef90722
> > --- /dev/null
> > +++ b/drivers/misc/ntsync.c
> > @@ -0,0 +1,52 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ntsync.c - Kernel driver for NT synchronization primitives
> > + *
> > + * Copyright (C) 2024 Elizabeth Figura
> 
> It would be nice to have your email address above...
> 
> > + */
> > +
> 
> > +
> > +MODULE_AUTHOR("Elizabeth Figura");
> 
> but at least please add it in MODULE_AUTHOR(). Yes it's optional, but
> roughly 7900 of 10400 entries do include it (according to my rough
> grepping).
> 
> Yes, I know that it's in MAINTAINERS.

Will add, thanks. As you guessed I did just pick an arbitrary module to copy.



^ permalink raw reply

* Re: [PATCH 02/31] ntsync: Introduce NTSYNC_IOC_CREATE_SEM.
From: Elizabeth Figura @ 2024-02-15 19:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Arnd Bergmann, Jonathan Corbet, Shuah Khan, linux-kernel,
	linux-api, wine-devel, André Almeida, Wolfram Sang,
	Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski, linux-doc,
	linux-kselftest
In-Reply-To: <2024021533-thicken-handed-7d2d@gregkh>

On Thursday, 15 February 2024 01:28:32 CST Greg Kroah-Hartman wrote:
> On Wed, Feb 14, 2024 at 05:36:38PM -0600, Elizabeth Figura wrote:
> > This corresponds to the NT syscall NtCreateSemaphore().
> > 
> > Semaphores are one of three types of object to be implemented in this driver,
> > the others being mutexes and events.
> > 
> > An NT semaphore contains a 32-bit counter, and is signaled and can be acquired
> > when the counter is nonzero. The counter has a maximum value which is specified
> > at creation time. The initial value of the semaphore is also specified at
> > creation time. There are no restrictions on the maximum and initial value.
> > 
> > Each object is exposed as an file, to which any number of fds may be opened.
> > When all fds are closed, the object is deleted.
> > 
> > Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
> > ---
> >  .../userspace-api/ioctl/ioctl-number.rst      |   2 +
> >  drivers/misc/ntsync.c                         | 120 ++++++++++++++++++
> >  include/uapi/linux/ntsync.h                   |  21 +++
> >  3 files changed, 143 insertions(+)
> >  create mode 100644 include/uapi/linux/ntsync.h
> > 
> > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > index 457e16f06e04..2f5c6994f042 100644
> > --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> > +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > @@ -173,6 +173,8 @@ Code  Seq#    Include File                                           Comments
> >  'M'   00-0F  drivers/video/fsl-diu-fb.h                              conflict!
> >  'N'   00-1F  drivers/usb/scanner.h
> >  'N'   40-7F  drivers/block/nvme.c
> > +'N'   80-8F  uapi/linux/ntsync.h                                     NT synchronization primitives
> > +                                                                     <mailto:wine-devel@winehq.org>
> >  'O'   00-06  mtd/ubi-user.h                                          UBI
> >  'P'   all    linux/soundcard.h                                       conflict!
> >  'P'   60-6F  sound/sscape_ioctl.h                                    conflict!
> > diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
> > index e4969ef90722..3ad86d98b82d 100644
> > --- a/drivers/misc/ntsync.c
> > +++ b/drivers/misc/ntsync.c
> > @@ -5,26 +5,146 @@
> >   * Copyright (C) 2024 Elizabeth Figura
> >   */
> >  
> > +#include <linux/anon_inodes.h>
> > +#include <linux/file.h>
> >  #include <linux/fs.h>
> >  #include <linux/miscdevice.h>
> >  #include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <uapi/linux/ntsync.h>
> >  
> >  #define NTSYNC_NAME	"ntsync"
> >  
> > +enum ntsync_type {
> > +	NTSYNC_TYPE_SEM,
> > +};
> > +
> > +struct ntsync_obj {
> > +	enum ntsync_type type;
> > +
> > +	union {
> > +		struct {
> > +			__u32 count;
> > +			__u32 max;
> > +		} sem;
> > +	} u;
> > +
> > +	struct file *file;
> > +	struct ntsync_device *dev;
> > +};
> > +
> > +struct ntsync_device {
> > +	struct file *file;
> > +};
> 
> No reference counting is needed for your ntsync_device?  Or are you
> relying on the reference counting of struct file here?
> 
> You pass around pointers to this structure, and save it off into other
> structures, how do you know it is "safe" to do so?

Yes, this relies on the reference counting of struct file. The sync
objects (semaphore etc.) grab a reference when they're created, via
get_file(), and release it when they're destroyed. This reference is
taken from within ioctls on the ntsync_device, so the file must be
valid when we grab a reference. Maybe I'm missing something, though?



^ permalink raw reply

* Re: [PATCH v2] uapi/auxvec: Define AT_HWCAP3 and AT_HWCAP4 aux vector, entries
From: Michael Ellerman @ 2024-02-16  1:49 UTC (permalink / raw)
  To: Peter Bergner, Arnd Bergmann, linux-api, Linux-Arch,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
  Cc: Adhemerval Zanella Netto, Szabolcs Nagy, Nick Piggin
In-Reply-To: <a50cf258-b861-40e5-8ca9-dec7721400ec@linux.ibm.com>

Peter Bergner <bergner@linux.ibm.com> writes:
> On 2/15/24 2:16 AM, Arnd Bergmann wrote:
>> On Wed, Feb 14, 2024, at 23:34, Peter Bergner wrote:
>>> The powerpc toolchain keeps a copy of the HWCAP bit masks in our TCB for fast
>>> access by the __builtin_cpu_supports built-in function.  The TCB space for
>>> the HWCAP entries - which are created in pairs - is an ABI extension, so
>>> waiting to create the space for HWCAP3 and HWCAP4 until we need them is
>>> problematical.  Define AT_HWCAP3 and AT_HWCAP4 in the generic uapi header
>>> so they can be used in glibc to reserve space in the powerpc TCB for their
>>> future use.
>>>
>>> I scanned through the Linux and GLIBC source codes looking for unused AT_*
>>> values and 29 and 30 did not seem to be used, so they are what I went
>>> with.  This has received Acked-by's from both GLIBC and Linux kernel
>>> developers and no reservations or Nacks from anyone.
>>>
>>> Arnd, we seem to have consensus on the patch below.  Is this something
>>> you could take and apply to your tree? 
>>>
>> 
>> I don't mind taking it, but it may be better to use the
>> powerpc tree if that is where it's actually being used.
>
> So this is not a powerpc only patch, but we may be the first arch
> to use it.  Szabolcs mentioned that aarch64 was pretty quickly filling
> up their AT_HWCAP2 and that they will eventually require using AT_HWCAP3
> as well.  If you still think this should go through the powerpc tree,
> I can check on that.

I'm happy to take it with Arnd's ack.

I trimmed up the commit message a bit, see below.

cheers


Author:     Peter Bergner <bergner@linux.ibm.com>
AuthorDate: Wed Feb 14 16:34:06 2024 -0600
Commit:     Michael Ellerman <mpe@ellerman.id.au>
CommitDate: Fri Feb 16 12:42:59 2024 +1100

    uapi/auxvec: Define AT_HWCAP3 and AT_HWCAP4 aux vector, entries

    The powerpc toolchain keeps a copy of the HWCAP bit masks in the TCB
    for fast access by the __builtin_cpu_supports() built-in function. The
    TCB space for the HWCAP entries - which are created in pairs - is an ABI
    extension, so waiting to create the space for HWCAP3 and HWCAP4 until
    they are needed is problematic. Define AT_HWCAP3 and AT_HWCAP4 in the
    generic uapi header so they can be used in glibc to reserve space in the
    powerpc TCB for their future use.

    I scanned through the Linux and GLIBC source codes looking for unused
    AT_* values and 29 and 30 did not seem to be used, so they are what I
    went with.

    Signed-off-by: Peter Bergner <bergner@linux.ibm.com>
    Acked-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
    Acked-by: Nicholas Piggin <npiggin@gmail.com>
    Acked-by: Szabolcs Nagy <szabolcs.nagy@arm.com>
    Acked-by: Arnd Bergmann <arnd@arndb.de>
    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
    Link: https://msgid.link/a406b535-dc55-4856-8ae9-5a063644a1af@linux.ibm.com

^ permalink raw reply

* Re: [PATCH v2] uapi/auxvec: Define AT_HWCAP3 and AT_HWCAP4 aux vector, entries
From: Peter Bergner @ 2024-02-16  4:13 UTC (permalink / raw)
  To: Michael Ellerman, Arnd Bergmann, linux-api, Linux-Arch,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
  Cc: Adhemerval Zanella Netto, Szabolcs Nagy, Nick Piggin
In-Reply-To: <87edddp48o.fsf@mail.lhotse>

On 2/15/24 7:49 PM, Michael Ellerman wrote:
> Peter Bergner <bergner@linux.ibm.com> writes:
>> On 2/15/24 2:16 AM, Arnd Bergmann wrote:
>>> On Wed, Feb 14, 2024, at 23:34, Peter Bergner wrote:
>>>> Arnd, we seem to have consensus on the patch below.  Is this something
>>>> you could take and apply to your tree? 
>>>>
>>>
>>> I don't mind taking it, but it may be better to use the
>>> powerpc tree if that is where it's actually being used.
>>
>> So this is not a powerpc only patch, but we may be the first arch
>> to use it.  Szabolcs mentioned that aarch64 was pretty quickly filling
>> up their AT_HWCAP2 and that they will eventually require using AT_HWCAP3
>> as well.  If you still think this should go through the powerpc tree,
>> I can check on that.
> 
> I'm happy to take it with Arnd's ack.
> 
> I trimmed up the commit message a bit, see below.

Perfect.  Thanks everyone!

Peter



^ permalink raw reply

* Re: [PATCH v2 2/2] pidfd: change pidfd_send_signal() to respect PIDFD_THREAD
From: Christian Brauner @ 2024-02-16 12:28 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
	linux-kernel
In-Reply-To: <20240214123655.GB16265@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

On Wed, Feb 14, 2024 at 01:36:56PM +0100, Oleg Nesterov wrote:
> On 02/10, Oleg Nesterov wrote:
> >
> > On 02/10, Christian Brauner wrote:
> > >
> > > +	if (type == PIDFD_SIGNAL_PROCESS_GROUP)
> > > +		ret = kill_pgrp_info(sig, &kinfo, pid);
> >
> > I guess you meant
> >
> > 	if (type == PIDTYPE_PGID)
> >
> > other than that,
> >
> > Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> 
> Yes, but there is another thing I hadn't thought of...
> 
> sys_pidfd_send_signal() does
> 
> 	/* Only allow sending arbitrary signals to yourself. */
> 	ret = -EPERM;
> 	if ((task_pid(current) != pid) &&
> 	    (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL))
> 		goto err;
> 
> and I am not sure that task_pid(current) == pid should allow
> the "arbitrary signals" if PIDFD_SIGNAL_PROCESS_GROUP.
> 
> Perhaps
> 
> 	/* Only allow sending arbitrary signals to yourself. */
> 	ret = -EPERM;
> 	if ((task_pid(current) != pid || type == PIDTYPE_PGID) &&
> 	    (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)
> 		goto err;

Honestly, we should probably just do:

if (kinfo->si_code != SI_USER)
        goto err

and be done with it. If we get regressions reports about this then it's
easy to fix that up. But I find that unlikely. So why not try to get
away with something much simpler. What do you think?

[-- Attachment #2: 0001-signal-disallow-non-SI_USER-signals-in-pidfd_send_si.patch --]
[-- Type: text/x-diff, Size: 1739 bytes --]

From 82a0d641e6f0bcf1a81731e06462df6911ecdd4e Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Fri, 16 Feb 2024 13:21:11 +0100
Subject: [PATCH] signal: disallow non-SI_USER signals in pidfd_send_signal()

Oleg pointed out that the following condition:

/* Only allow sending arbitrary signals to yourself. */
ret = -EPERM;
if ((task_pid(current) != pid) &&
    (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL))
        goto err;

doesn't account for PIDFD_SIGNAL_PROCESS_GROUP. He suggested:

/* Only allow sending arbitrary signals to yourself. */
ret = -EPERM;
if ((task_pid(current) != pid || type == PIDTYPE_PGID) &&
    (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)
        goto err;

but I think we should just go all the way and error out if userspace
specifies anything else than SI_USER as si_code. It's probably an unused
feature right now anyway and if someone needs it than it's easy to add
back.

Reported-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20240214123655.GB16265@redhat.com
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 kernel/signal.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index cf6539a6b1cb..92a80e8d6b22 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3954,10 +3954,7 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
 		if (unlikely(sig != kinfo.si_signo))
 			goto err;
 
-		/* Only allow sending arbitrary signals to yourself. */
-		ret = -EPERM;
-		if ((task_pid(current) != pid) &&
-		    (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL))
+		if (kinfo.si_code != SI_USER)
 			goto err;
 	} else {
 		prepare_kill_siginfo(sig, &kinfo, type);
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v2 2/2] pidfd: change pidfd_send_signal() to respect PIDFD_THREAD
From: Oleg Nesterov @ 2024-02-16 13:06 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
	linux-kernel
In-Reply-To: <20240216-albern-aufwiegen-1de327c7dafd@brauner>

On 02/16, Christian Brauner wrote:
>
> On Wed, Feb 14, 2024 at 01:36:56PM +0100, Oleg Nesterov wrote:
> >
> > and I am not sure that task_pid(current) == pid should allow
> > the "arbitrary signals" if PIDFD_SIGNAL_PROCESS_GROUP.
> >
> > Perhaps
> >
> > 	/* Only allow sending arbitrary signals to yourself. */
> > 	ret = -EPERM;
> > 	if ((task_pid(current) != pid || type == PIDTYPE_PGID) &&
> > 	    (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)
> > 		goto err;
>
> Honestly, we should probably just do:
>
> if (kinfo->si_code != SI_USER)
>         goto err

Hmm. This doesn't look right. The purpose of the current check is to
forbid SI_TKILL and si_code >= 0, and SI_USER == 0.

SI_USER means that the target can trust the values of si_pid/si_uid
in siginfo.

> +		if (kinfo.si_code != SI_USER)
>  			goto err;

See above...

Oleg.


^ permalink raw reply

* Re: [PATCH v2 2/2] pidfd: change pidfd_send_signal() to respect PIDFD_THREAD
From: Christian Brauner @ 2024-02-16 14:46 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Andy Lutomirski, Eric W. Biederman, Tycho Andersen, linux-api,
	linux-kernel
In-Reply-To: <20240216130625.GA8723@redhat.com>

> SI_USER means that the target can trust the values of si_pid/si_uid
> in siginfo.

Bah, what an annoying nonsense. I see that this can be used to emulate
stuff like SI_TIMER and SI_ASYNCIO. But I very much doubt the value of
e.g., emulating SI_DETHREAD. Maybe I'm missing something very obvious.
In any case, thanks for keeping me honest:

So wouldn't be better of just writing this as?

if ((task_pid(current) != pid || type > PIDTYPE_TGID) &&
    (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL))
        goto err;

So that we don't have to repeat the same exercise if we extend this to
anything above PIDTYPE_PGID?

^ permalink raw reply

* Re: [PATCH 00/31] NT synchronization primitive driver
From: Alexey Dobriyan @ 2024-02-16 16:31 UTC (permalink / raw)
  To: Elizabeth Figura
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan,
	linux-kernel, linux-api, André Almeida, Wolfram Sang,
	Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
	Elizabeth Figura, linux-doc, linux-kselftest

> drivers/misc/ntsync.c                         | 1146 ++++++++++++++

Assuming this doesn't go into futex(2) or some other existing code...

Can you start putting all of this into top-level "windows" directory?
I suspect there will be more Windows stuff in the future.

So those who don't care about Windows can turn off just one config option
(CONFIG_WINDOWS) and be done with it.

Name it "Linux Subsystem for Windows" for 146% better memes.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox