* [PATCH v6 15/28] selftests: ntsync: Add some tests for mutex state.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
Test mutex-specific ioctls NTSYNC_IOC_MUTEX_UNLOCK and NTSYNC_IOC_MUTEX_READ,
and waiting on mutexes.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 196 ++++++++++++++++++
1 file changed, 196 insertions(+)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 1e145c6dfded..7cd0f40594fd 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -40,6 +40,39 @@ static int post_sem(int sem, __u32 *count)
return ioctl(sem, NTSYNC_IOC_SEM_POST, count);
}
+static int read_mutex_state(int mutex, __u32 *count, __u32 *owner)
+{
+ struct ntsync_mutex_args args;
+ int ret;
+
+ memset(&args, 0xcc, sizeof(args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &args);
+ *count = args.count;
+ *owner = args.owner;
+ return ret;
+}
+
+#define check_mutex_state(mutex, count, owner) \
+ ({ \
+ __u32 __count, __owner; \
+ int ret = read_mutex_state((mutex), &__count, &__owner); \
+ EXPECT_EQ(0, ret); \
+ EXPECT_EQ((count), __count); \
+ EXPECT_EQ((owner), __owner); \
+ })
+
+static int unlock_mutex(int mutex, __u32 owner, __u32 *count)
+{
+ struct ntsync_mutex_args args;
+ int ret;
+
+ args.owner = owner;
+ args.count = 0xdeadbeef;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_UNLOCK, &args);
+ *count = args.count;
+ return ret;
+}
+
static int wait_any(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
{
struct ntsync_wait_args args = {0};
@@ -146,4 +179,167 @@ TEST(semaphore_state)
close(fd);
}
+TEST(mutex_state)
+{
+ struct ntsync_mutex_args mutex_args;
+ __u32 owner, count, index;
+ struct timespec timeout;
+ int fd, ret, mutex;
+
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ mutex_args.owner = 123;
+ mutex_args.count = 0;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ mutex_args.owner = 0;
+ mutex_args.count = 2;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ mutex_args.owner = 123;
+ mutex_args.count = 2;
+ mutex_args.mutex = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ mutex = mutex_args.mutex;
+ check_mutex_state(mutex, 2, 123);
+
+ ret = unlock_mutex(mutex, 0, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ ret = unlock_mutex(mutex, 456, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EPERM, errno);
+ check_mutex_state(mutex, 2, 123);
+
+ ret = unlock_mutex(mutex, 123, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, count);
+ check_mutex_state(mutex, 1, 123);
+
+ ret = unlock_mutex(mutex, 123, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, count);
+ check_mutex_state(mutex, 0, 0);
+
+ ret = unlock_mutex(mutex, 123, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EPERM, errno);
+
+ ret = wait_any(fd, 1, &mutex, 456, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 456);
+
+ ret = wait_any(fd, 1, &mutex, 456, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 2, 456);
+
+ ret = unlock_mutex(mutex, 456, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, count);
+ check_mutex_state(mutex, 1, 456);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ owner = 0;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ owner = 123;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EPERM, errno);
+ check_mutex_state(mutex, 1, 456);
+
+ owner = 456;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(0, ret);
+
+ memset(&mutex_args, 0xcc, sizeof(mutex_args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, mutex_args.count);
+ EXPECT_EQ(0, mutex_args.owner);
+
+ memset(&mutex_args, 0xcc, sizeof(mutex_args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, mutex_args.count);
+ EXPECT_EQ(0, mutex_args.owner);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 123);
+
+ owner = 123;
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(0, ret);
+
+ memset(&mutex_args, 0xcc, sizeof(mutex_args));
+ ret = ioctl(mutex, NTSYNC_IOC_MUTEX_READ, &mutex_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, mutex_args.count);
+ EXPECT_EQ(0, mutex_args.owner);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 123);
+
+ close(mutex);
+
+ mutex_args.owner = 0;
+ mutex_args.count = 0;
+ mutex_args.mutex = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ mutex = mutex_args.mutex;
+ check_mutex_state(mutex, 0, 0);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_mutex_state(mutex, 1, 123);
+
+ close(mutex);
+
+ mutex_args.owner = 123;
+ mutex_args.count = ~0u;
+ mutex_args.mutex = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ mutex = mutex_args.mutex;
+ check_mutex_state(mutex, ~0u, 123);
+
+ ret = wait_any(fd, 1, &mutex, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ close(mutex);
+
+ close(fd);
+}
+
TEST_HARNESS_MAIN
--
2.45.2
^ permalink raw reply related
* [PATCH v6 11/28] ntsync: Introduce NTSYNC_IOC_MUTEX_READ.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtQueryMutant().
This returns the recursion count, owner, and abandoned state of the mutex.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 28 ++++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 1 +
2 files changed, 29 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index d6e8a4bde1d0..cff2627c1efe 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -607,6 +607,32 @@ static int ntsync_sem_read(struct ntsync_obj *sem, void __user *argp)
return 0;
}
+static int ntsync_mutex_read(struct ntsync_obj *mutex, void __user *argp)
+{
+ struct ntsync_mutex_args __user *user_args = argp;
+ struct ntsync_device *dev = mutex->dev;
+ struct ntsync_mutex_args args;
+ bool all;
+ int ret;
+
+ if (mutex->type != NTSYNC_TYPE_MUTEX)
+ return -EINVAL;
+
+ args.mutex = 0;
+
+ all = ntsync_lock_obj(dev, mutex);
+
+ args.count = mutex->u.mutex.count;
+ args.owner = mutex->u.mutex.owner;
+ ret = mutex->u.mutex.ownerdead ? -EOWNERDEAD : 0;
+
+ ntsync_unlock_obj(dev, mutex, all);
+
+ if (copy_to_user(user_args, &args, sizeof(args)))
+ return -EFAULT;
+ return ret;
+}
+
static int ntsync_obj_release(struct inode *inode, struct file *file)
{
struct ntsync_obj *obj = file->private_data;
@@ -632,6 +658,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
return ntsync_mutex_unlock(obj, argp);
case NTSYNC_IOC_MUTEX_KILL:
return ntsync_mutex_kill(obj, argp);
+ case NTSYNC_IOC_MUTEX_READ:
+ return ntsync_mutex_read(obj, argp);
case NTSYNC_IOC_EVENT_SET:
return ntsync_event_set(obj, argp, false);
case NTSYNC_IOC_EVENT_RESET:
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 5e922703686f..eced73d08783 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -55,5 +55,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_EVENT_RESET _IOR ('N', 0x89, __u32)
#define NTSYNC_IOC_EVENT_PULSE _IOR ('N', 0x8a, __u32)
#define NTSYNC_IOC_SEM_READ _IOR ('N', 0x8b, struct ntsync_sem_args)
+#define NTSYNC_IOC_MUTEX_READ _IOR ('N', 0x8c, struct ntsync_mutex_args)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 12/28] ntsync: Introduce NTSYNC_IOC_EVENT_READ.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
This corresponds to the NT syscall NtQueryEvent().
This returns the signaled state of the event and whether it is manual-reset.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 26 ++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 1 +
2 files changed, 27 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index cff2627c1efe..5a5ee7b6ee92 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -633,6 +633,30 @@ static int ntsync_mutex_read(struct ntsync_obj *mutex, void __user *argp)
return ret;
}
+static int ntsync_event_read(struct ntsync_obj *event, void __user *argp)
+{
+ struct ntsync_event_args __user *user_args = argp;
+ struct ntsync_device *dev = event->dev;
+ struct ntsync_event_args args;
+ bool all;
+
+ if (event->type != NTSYNC_TYPE_EVENT)
+ return -EINVAL;
+
+ args.event = 0;
+
+ all = ntsync_lock_obj(dev, event);
+
+ args.manual = event->u.event.manual;
+ args.signaled = event->u.event.signaled;
+
+ ntsync_unlock_obj(dev, event, all);
+
+ if (copy_to_user(user_args, &args, sizeof(args)))
+ return -EFAULT;
+ return 0;
+}
+
static int ntsync_obj_release(struct inode *inode, struct file *file)
{
struct ntsync_obj *obj = file->private_data;
@@ -666,6 +690,8 @@ static long ntsync_obj_ioctl(struct file *file, unsigned int cmd,
return ntsync_event_reset(obj, argp);
case NTSYNC_IOC_EVENT_PULSE:
return ntsync_event_set(obj, argp, true);
+ case NTSYNC_IOC_EVENT_READ:
+ return ntsync_event_read(obj, argp);
default:
return -ENOIOCTLCMD;
}
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index eced73d08783..74abeba832f7 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -56,5 +56,6 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_EVENT_PULSE _IOR ('N', 0x8a, __u32)
#define NTSYNC_IOC_SEM_READ _IOR ('N', 0x8b, struct ntsync_sem_args)
#define NTSYNC_IOC_MUTEX_READ _IOR ('N', 0x8c, struct ntsync_mutex_args)
+#define NTSYNC_IOC_EVENT_READ _IOR ('N', 0x8d, struct ntsync_event_args)
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v6 13/28] ntsync: Introduce alertable waits.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
NT waits can optionally be made "alertable". This is a special channel for
thread wakeup that is mildly similar to SIGIO. A thread has an internal single
bit of "alerted" state, and if a thread is alerted while an alertable wait, the
wait will return a special value, consume the "alerted" state, and will not
consume any of its objects.
Alerts are implemented using events; the user-space NT emulator is expected to
create an internal ntsync event for each thread and pass that event to wait
functions.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 70 ++++++++++++++++++++++++++++++++-----
include/uapi/linux/ntsync.h | 3 +-
2 files changed, 63 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 5a5ee7b6ee92..3fac06270549 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -884,22 +884,29 @@ static int setup_wait(struct ntsync_device *dev,
const struct ntsync_wait_args *args, bool all,
struct ntsync_q **ret_q)
{
+ int fds[NTSYNC_MAX_WAIT_COUNT + 1];
const __u32 count = args->count;
- int fds[NTSYNC_MAX_WAIT_COUNT];
struct ntsync_q *q;
+ __u32 total_count;
__u32 i, j;
- if (args->pad[0] || args->pad[1] || (args->flags & ~NTSYNC_WAIT_REALTIME))
+ if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME))
return -EINVAL;
if (args->count > NTSYNC_MAX_WAIT_COUNT)
return -EINVAL;
+ total_count = count;
+ if (args->alert)
+ total_count++;
+
if (copy_from_user(fds, u64_to_user_ptr(args->objs),
array_size(count, sizeof(*fds))))
return -EFAULT;
+ if (args->alert)
+ fds[count] = args->alert;
- q = kmalloc(struct_size(q, entries, count), GFP_KERNEL);
+ q = kmalloc(struct_size(q, entries, total_count), GFP_KERNEL);
if (!q)
return -ENOMEM;
q->task = current;
@@ -909,7 +916,7 @@ static int setup_wait(struct ntsync_device *dev,
q->ownerdead = false;
q->count = count;
- for (i = 0; i < count; i++) {
+ for (i = 0; i < total_count; i++) {
struct ntsync_q_entry *entry = &q->entries[i];
struct ntsync_obj *obj = get_obj(dev, fds[i]);
@@ -959,10 +966,10 @@ static void try_wake_any_obj(struct ntsync_obj *obj)
static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
{
struct ntsync_wait_args args;
+ __u32 i, total_count;
struct ntsync_q *q;
int signaled;
bool all;
- __u32 i;
int ret;
if (copy_from_user(&args, argp, sizeof(args)))
@@ -972,9 +979,13 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
if (ret < 0)
return ret;
+ total_count = args.count;
+ if (args.alert)
+ total_count++;
+
/* queue ourselves */
- for (i = 0; i < args.count; i++) {
+ for (i = 0; i < total_count; i++) {
struct ntsync_q_entry *entry = &q->entries[i];
struct ntsync_obj *obj = entry->obj;
@@ -983,9 +994,15 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
ntsync_unlock_obj(dev, obj, all);
}
- /* check if we are already signaled */
+ /*
+ * Check if we are already signaled.
+ *
+ * Note that the API requires that normal objects are checked before
+ * the alert event. Hence we queue the alert event last, and check
+ * objects in order.
+ */
- for (i = 0; i < args.count; i++) {
+ for (i = 0; i < total_count; i++) {
struct ntsync_obj *obj = q->entries[i].obj;
if (atomic_read(&q->signaled) != -1)
@@ -1002,7 +1019,7 @@ static int ntsync_wait_any(struct ntsync_device *dev, void __user *argp)
/* and finally, unqueue */
- for (i = 0; i < args.count; i++) {
+ for (i = 0; i < total_count; i++) {
struct ntsync_q_entry *entry = &q->entries[i];
struct ntsync_obj *obj = entry->obj;
@@ -1062,6 +1079,14 @@ static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp)
*/
list_add_tail(&entry->node, &obj->all_waiters);
}
+ if (args.alert) {
+ struct ntsync_q_entry *entry = &q->entries[args.count];
+ struct ntsync_obj *obj = entry->obj;
+
+ dev_lock_obj(dev, obj);
+ list_add_tail(&entry->node, &obj->any_waiters);
+ dev_unlock_obj(dev, obj);
+ }
/* check if we are already signaled */
@@ -1069,6 +1094,21 @@ static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp)
mutex_unlock(&dev->wait_all_lock);
+ /*
+ * Check if the alert event is signaled, making sure to do so only
+ * after checking if the other objects are signaled.
+ */
+
+ if (args.alert) {
+ struct ntsync_obj *obj = q->entries[args.count].obj;
+
+ if (atomic_read(&q->signaled) == -1) {
+ bool all = ntsync_lock_obj(dev, obj);
+ try_wake_any_obj(obj);
+ ntsync_unlock_obj(dev, obj, all);
+ }
+ }
+
/* sleep */
ret = ntsync_schedule(q, &args);
@@ -1094,6 +1134,18 @@ static int ntsync_wait_all(struct ntsync_device *dev, void __user *argp)
mutex_unlock(&dev->wait_all_lock);
+ if (args.alert) {
+ struct ntsync_q_entry *entry = &q->entries[args.count];
+ struct ntsync_obj *obj = entry->obj;
+ bool all;
+
+ all = ntsync_lock_obj(dev, obj);
+ list_del(&entry->node);
+ ntsync_unlock_obj(dev, obj, all);
+
+ put_obj(obj);
+ }
+
signaled = atomic_read(&q->signaled);
if (signaled != -1) {
struct ntsync_wait_args __user *user_args = argp;
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 74abeba832f7..4a8095a3fc34 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -37,7 +37,8 @@ struct ntsync_wait_args {
__u32 index;
__u32 flags;
__u32 owner;
- __u32 pad[2];
+ __u32 alert;
+ __u32 pad;
};
#define NTSYNC_MAX_WAIT_COUNT 64
--
2.45.2
^ permalink raw reply related
* [PATCH v6 14/28] selftests: ntsync: Add some tests for semaphore state.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
Wine has tests for its synchronization primitives, but these are more accessible
to kernel developers, and also allow us to test some edge cases that Wine does
not care about.
This patch adds tests for semaphore-specific ioctls NTSYNC_IOC_SEM_POST and
NTSYNC_IOC_SEM_READ, and waiting on semaphores.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
tools/testing/selftests/Makefile | 1 +
.../selftests/drivers/ntsync/.gitignore | 1 +
.../testing/selftests/drivers/ntsync/Makefile | 7 +
tools/testing/selftests/drivers/ntsync/config | 1 +
.../testing/selftests/drivers/ntsync/ntsync.c | 149 ++++++++++++++++++
5 files changed, 159 insertions(+)
create mode 100644 tools/testing/selftests/drivers/ntsync/.gitignore
create mode 100644 tools/testing/selftests/drivers/ntsync/Makefile
create mode 100644 tools/testing/selftests/drivers/ntsync/config
create mode 100644 tools/testing/selftests/drivers/ntsync/ntsync.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 2401e973c359..a8c9648e5adc 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -18,6 +18,7 @@ TARGETS += devices/error_logs
TARGETS += devices/probe
TARGETS += dmabuf-heaps
TARGETS += drivers/dma-buf
+TARGETS += drivers/ntsync
TARGETS += drivers/s390x/uvdevice
TARGETS += drivers/net
TARGETS += drivers/net/bonding
diff --git a/tools/testing/selftests/drivers/ntsync/.gitignore b/tools/testing/selftests/drivers/ntsync/.gitignore
new file mode 100644
index 000000000000..848573a3d3ea
--- /dev/null
+++ b/tools/testing/selftests/drivers/ntsync/.gitignore
@@ -0,0 +1 @@
+ntsync
diff --git a/tools/testing/selftests/drivers/ntsync/Makefile b/tools/testing/selftests/drivers/ntsync/Makefile
new file mode 100644
index 000000000000..dbf2b055c0b2
--- /dev/null
+++ b/tools/testing/selftests/drivers/ntsync/Makefile
@@ -0,0 +1,7 @@
+# SPDX-LICENSE-IDENTIFIER: GPL-2.0-only
+TEST_GEN_PROGS := ntsync
+
+CFLAGS += $(KHDR_INCLUDES)
+LDLIBS += -lpthread
+
+include ../../lib.mk
diff --git a/tools/testing/selftests/drivers/ntsync/config b/tools/testing/selftests/drivers/ntsync/config
new file mode 100644
index 000000000000..60539c826d06
--- /dev/null
+++ b/tools/testing/selftests/drivers/ntsync/config
@@ -0,0 +1 @@
+CONFIG_WINESYNC=y
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
new file mode 100644
index 000000000000..1e145c6dfded
--- /dev/null
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Various unit tests for the "ntsync" synchronization primitive driver.
+ *
+ * Copyright (C) 2021-2022 Elizabeth Figura <zfigura@codeweavers.com>
+ */
+
+#define _GNU_SOURCE
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <time.h>
+#include <pthread.h>
+#include <linux/ntsync.h>
+#include "../../kselftest_harness.h"
+
+static int read_sem_state(int sem, __u32 *count, __u32 *max)
+{
+ struct ntsync_sem_args args;
+ int ret;
+
+ memset(&args, 0xcc, sizeof(args));
+ ret = ioctl(sem, NTSYNC_IOC_SEM_READ, &args);
+ *count = args.count;
+ *max = args.max;
+ return ret;
+}
+
+#define check_sem_state(sem, count, max) \
+ ({ \
+ __u32 __count, __max; \
+ int ret = read_sem_state((sem), &__count, &__max); \
+ EXPECT_EQ(0, ret); \
+ EXPECT_EQ((count), __count); \
+ EXPECT_EQ((max), __max); \
+ })
+
+static int post_sem(int sem, __u32 *count)
+{
+ return ioctl(sem, NTSYNC_IOC_SEM_POST, count);
+}
+
+static int wait_any(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
+{
+ struct ntsync_wait_args args = {0};
+ struct timespec timeout;
+ int ret;
+
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+
+ args.timeout = timeout.tv_sec * 1000000000 + timeout.tv_nsec;
+ args.count = count;
+ args.objs = (uintptr_t)objs;
+ args.owner = owner;
+ args.index = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_WAIT_ANY, &args);
+ *index = args.index;
+ return ret;
+}
+
+TEST(semaphore_state)
+{
+ struct ntsync_sem_args sem_args;
+ struct timespec timeout;
+ __u32 count, index;
+ int fd, ret, sem;
+
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ sem_args.count = 3;
+ sem_args.max = 2;
+ sem_args.sem = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ 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);
+ sem = sem_args.sem;
+ check_sem_state(sem, 2, 2);
+
+ count = 0;
+ ret = post_sem(sem, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, count);
+ check_sem_state(sem, 2, 2);
+
+ count = 1;
+ ret = post_sem(sem, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOVERFLOW, errno);
+ check_sem_state(sem, 2, 2);
+
+ ret = wait_any(fd, 1, &sem, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem, 1, 2);
+
+ ret = wait_any(fd, 1, &sem, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem, 0, 2);
+
+ ret = wait_any(fd, 1, &sem, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ count = 3;
+ ret = post_sem(sem, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOVERFLOW, errno);
+ check_sem_state(sem, 0, 2);
+
+ count = 2;
+ ret = post_sem(sem, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, count);
+ check_sem_state(sem, 2, 2);
+
+ ret = wait_any(fd, 1, &sem, 123, &index);
+ EXPECT_EQ(0, ret);
+ ret = wait_any(fd, 1, &sem, 123, &index);
+ EXPECT_EQ(0, ret);
+
+ count = 1;
+ ret = post_sem(sem, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, count);
+ check_sem_state(sem, 1, 2);
+
+ count = ~0u;
+ ret = post_sem(sem, &count);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOVERFLOW, errno);
+ check_sem_state(sem, 1, 2);
+
+ close(sem);
+
+ close(fd);
+}
+
+TEST_HARNESS_MAIN
--
2.45.2
^ permalink raw reply related
* [PATCH v6 27/28] docs: ntsync: Add documentation for the ntsync uAPI.
From: Elizabeth Figura @ 2024-12-09 18:59 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 | 398 +++++++++++++++++++++++++
2 files changed, 399 insertions(+)
create mode 100644 Documentation/userspace-api/ntsync.rst
diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index 274cc7546efc..9c1b15cd89ab 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -63,6 +63,7 @@ Everything else
vduse
futex2
perf_ring_buffer
+ 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..767844637a7d
--- /dev/null
+++ b/Documentation/userspace-api/ntsync.rst
@@ -0,0 +1,398 @@
+===================================
+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 (that is,
+can be acquired without contention, or will wake up a waiting thread)
+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 is similar to a semaphore with a maximum count of one. It 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.
+
+ 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.
+
+ 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.45.2
^ permalink raw reply related
* [PATCH v6 28/28] ntsync: No longer depend on BROKEN.
From: Elizabeth Figura @ 2024-12-09 18:59 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
f5b335dc025cfee90957efa90dc72fada0d5abb4 ("misc: ntsync: mark driver as "broken"
to prevent from building") was committed to avoid the driver being used while
only part of its functionality was released. Since the rest of the functionality
has now been committed, revert this.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 09cbe3f0ab1e..fb772bfe27c3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -517,7 +517,6 @@ config OPEN_DICE
config NTSYNC
tristate "NT synchronization primitive emulation"
- depends on BROKEN
help
This module provides kernel support for emulation of Windows NT
synchronization primitives. It is not a hardware driver.
--
2.45.2
^ permalink raw reply related
* [PATCH v6 26/28] maintainers: Add an entry for ntsync.
From: Elizabeth Figura @ 2024-12-09 18:59 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 1e930c7a58b1..8c3dd9077fc2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16708,6 +16708,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.45.2
^ permalink raw reply related
* [PATCH v6 24/28] selftests: ntsync: Add some tests for wakeup signaling via alerts.
From: Elizabeth Figura @ 2024-12-09 18:59 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 5465a16d38b3..968874d7e325 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.45.2
^ permalink raw reply related
* [PATCH v6 25/28] selftests: ntsync: Add a stress test for contended waits.
From: Elizabeth Figura @ 2024-12-09 18:59 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 968874d7e325..5fa2c9a0768c 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.45.2
^ permalink raw reply related
* [PATCH v6 23/28] selftests: ntsync: Add tests for alertable waits.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 5d17eff6a370..5465a16d38b3 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.45.2
^ permalink raw reply related
* [PATCH v6 21/28] selftests: ntsync: Add some tests for auto-reset event state.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 b6481c2b85cc..12ccb4ec28e4 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.45.2
^ permalink raw reply related
* [PATCH v6 22/28] selftests: ntsync: Add some tests for wakeup signaling with events.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 12ccb4ec28e4..5d17eff6a370 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.45.2
^ permalink raw reply related
* [PATCH v6 20/28] selftests: ntsync: Add some tests for manual-reset event state.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 b77fb0b2c4b1..b6481c2b85cc 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.45.2
^ permalink raw reply related
* [PATCH v6 19/28] selftests: ntsync: Add some tests for wakeup signaling with WINESYNC_IOC_WAIT_ALL.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-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 993f5db23768..b77fb0b2c4b1 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.45.2
^ permalink raw reply related
* [PATCH v6 18/28] selftests: ntsync: Add some tests for wakeup signaling with WINESYNC_IOC_WAIT_ANY.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
Test contended "wait-for-any" waits, to make sure that scheduling and wakeup
logic works correctly.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 150 ++++++++++++++++++
1 file changed, 150 insertions(+)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index c0f372167557..993f5db23768 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -556,4 +556,154 @@ TEST(test_wait_all)
close(fd);
}
+struct wake_args {
+ int fd;
+ int obj;
+};
+
+struct wait_args {
+ int fd;
+ unsigned long request;
+ struct ntsync_wait_args *args;
+ int ret;
+ int err;
+};
+
+static void *wait_thread(void *arg)
+{
+ struct wait_args *args = arg;
+
+ args->ret = ioctl(args->fd, args->request, args->args);
+ args->err = errno;
+ return NULL;
+}
+
+static __u64 get_abs_timeout(unsigned int ms)
+{
+ struct timespec timeout;
+ clock_gettime(CLOCK_MONOTONIC, &timeout);
+ return (timeout.tv_sec * 1000000000) + timeout.tv_nsec + (ms * 1000000);
+}
+
+static int wait_for_thread(pthread_t thread, unsigned int ms)
+{
+ struct timespec timeout;
+
+ clock_gettime(CLOCK_REALTIME, &timeout);
+ timeout.tv_nsec += ms * 1000000;
+ timeout.tv_sec += (timeout.tv_nsec / 1000000000);
+ timeout.tv_nsec %= 1000000000;
+ return pthread_timedjoin_np(thread, NULL, &timeout);
+}
+
+TEST(wake_any)
+{
+ 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;
+
+ /* test waking the semaphore */
+
+ wait_args.timeout = get_abs_timeout(1000);
+ wait_args.objs = (uintptr_t)objs;
+ wait_args.count = 2;
+ wait_args.owner = 456;
+ wait_args.index = 0xdeadbeef;
+ 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);
+
+ count = 1;
+ ret = post_sem(sem_args.sem, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, count);
+ check_sem_state(sem_args.sem, 0, 3);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(0, wait_args.index);
+
+ /* test waking the mutex */
+
+ /* first grab it again for owner 123 */
+ ret = wait_any(fd, 1, &mutex_args.mutex, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+
+ wait_args.timeout = get_abs_timeout(1000);
+ wait_args.owner = 456;
+ ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(ETIMEDOUT, ret);
+
+ ret = unlock_mutex(mutex_args.mutex, 123, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, count);
+
+ ret = pthread_tryjoin_np(thread, NULL);
+ EXPECT_EQ(EBUSY, ret);
+
+ ret = unlock_mutex(mutex_args.mutex, 123, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, mutex_args.count);
+ check_mutex_state(mutex_args.mutex, 1, 456);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(1, wait_args.index);
+
+ /* 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.45.2
^ permalink raw reply related
* [PATCH v6 17/28] selftests: ntsync: Add some tests for NTSYNC_IOC_WAIT_ALL.
From: Elizabeth Figura @ 2024-12-09 18:58 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, Randy Dunlap, Ingo Molnar,
Will Deacon, Waiman Long, Boqun Feng, Elizabeth Figura
In-Reply-To: <20241209185904.507350-1-zfigura@codeweavers.com>
Test basic synchronous functionality of NTSYNC_IOC_WAIT_ALL, and when objects
are considered simultaneously signaled.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 99 ++++++++++++++++++-
1 file changed, 97 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 40ad8cbd3138..c0f372167557 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -73,7 +73,8 @@ static int unlock_mutex(int mutex, __u32 owner, __u32 *count)
return ret;
}
-static int wait_any(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
+static int wait_objs(int fd, unsigned long request, __u32 count,
+ const int *objs, __u32 owner, __u32 *index)
{
struct ntsync_wait_args args = {0};
struct timespec timeout;
@@ -86,11 +87,21 @@ static int wait_any(int fd, __u32 count, const int *objs, __u32 owner, __u32 *in
args.objs = (uintptr_t)objs;
args.owner = owner;
args.index = 0xdeadbeef;
- ret = ioctl(fd, NTSYNC_IOC_WAIT_ANY, &args);
+ ret = ioctl(fd, request, &args);
*index = args.index;
return ret;
}
+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);
+}
+
+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);
+}
+
TEST(semaphore_state)
{
struct ntsync_sem_args sem_args;
@@ -461,4 +472,88 @@ TEST(test_wait_any)
close(fd);
}
+TEST(test_wait_all)
+{
+ struct ntsync_mutex_args mutex_args = {0};
+ struct ntsync_sem_args sem_args = {0};
+ __u32 owner, index, count;
+ int objs[2], fd, ret;
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ sem_args.count = 2;
+ 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 = 0;
+ mutex_args.count = 0;
+ 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;
+
+ ret = wait_all(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem_args.sem, 1, 3);
+ check_mutex_state(mutex_args.mutex, 1, 123);
+
+ ret = wait_all(fd, 2, objs, 456, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+ check_sem_state(sem_args.sem, 1, 3);
+ check_mutex_state(mutex_args.mutex, 1, 123);
+
+ 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_mutex_state(mutex_args.mutex, 2, 123);
+
+ ret = wait_all(fd, 2, objs, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+ check_sem_state(sem_args.sem, 0, 3);
+ check_mutex_state(mutex_args.mutex, 2, 123);
+
+ count = 3;
+ ret = post_sem(sem_args.sem, &count);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, count);
+
+ ret = wait_all(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem_args.sem, 2, 3);
+ check_mutex_state(mutex_args.mutex, 3, 123);
+
+ owner = 123;
+ ret = ioctl(mutex_args.mutex, NTSYNC_IOC_MUTEX_KILL, &owner);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_all(fd, 2, objs, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EOWNERDEAD, errno);
+ check_sem_state(sem_args.sem, 1, 3);
+ check_mutex_state(mutex_args.mutex, 1, 123);
+
+ /* test waiting on the same object twice */
+ objs[0] = objs[1] = sem_args.sem;
+ ret = wait_all(fd, 2, objs, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EINVAL, errno);
+
+ close(sem_args.sem);
+ close(mutex_args.mutex);
+
+ close(fd);
+}
+
TEST_HARNESS_MAIN
--
2.45.2
^ permalink raw reply related
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: John Ousterhout @ 2024-12-09 17:03 UTC (permalink / raw)
To: D. Wythe; +Cc: netdev, linux-api
In-Reply-To: <CAGXJAmxkR7Wgmd2+eUKwOHos2tGbSJ8UFqYKE4nsegg7sr96WQ@mail.gmail.com>
A follow-up question on this, if I may. Is it OK to vmap a large
region of user address space (say, 64 MB) and leave this mapped for an
extended period of time (say, the life of the application), or would
this have undesirable consequences? In other words, if I do this,
would I need to monitor how actively the memory is being used and
release the vmap for space that is inactive?
-John-
On Mon, Dec 9, 2024 at 8:53 AM John Ousterhout <ouster@cs.stanford.edu> wrote:
>
> Thanks for the additional information; I'll put this on my list of
> things to consider for performance optimization.
>
> -John-
>
>
> On Sun, Dec 8, 2024 at 10:56 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> >
> >
> >
> > On 12/6/24 3:49 AM, John Ousterhout wrote:
> > > On Sun, Dec 1, 2024 at 7:51 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > >>> +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
> > >>> + unsigned int optlen)
> > >>> +{
> > >>> + struct homa_sock *hsk = homa_sk(sk);
> > >>> + struct homa_set_buf_args args;
> > >>> + int ret;
> > >>> +
> > >>> + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
> > >>> + optlen != sizeof(struct homa_set_buf_args))
> > >>> + return -EINVAL;
> > >>
> > >> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
> > >> implemented in getsockopt.
> > >
> > > I have changed it to HOMA_RCVBUF (and renamed struct homa_set_buf_args
> > > to struct homa_rcvbuf_args). I also implemented getsockopt for
> > > HOMA_RCVBUF.
> > >
> > >>> +
> > >>> + if (copy_from_sockptr(&args, optval, optlen))
> > >>> + return -EFAULT;
> > >>> +
> > >>> + /* Do a trivial test to make sure we can at least write the first
> > >>> + * page of the region.
> > >>> + */
> > >>> + if (copy_to_user((__force void __user *)args.start, &args, sizeof(args)))
> > >>> + return -EFAULT;
> > >>
> > >> To share buffer between kernel and userspace, maybe you should refer to the implementation of
> > >> io_pin_pbuf_ring()
> > >
> > > I'm not sure what you mean here. Are you suggesting that I look at the
> > > code of io_pin_pbuf_ring to make sure I've done everything needed to
> > > share buffers? I don't believe that Homa needs to do anything special
> > > (e.g. it doesn't need to pin the user's buffers); it just saves the
> > > address and makes copy_to_user calls later when needed (and these
> > > calls are all done at syscall level in the context of the
> > > application). Is there something I'm missing?
> > >
> >
> > I just thought that since the received buffer is shared between kernel and user-space, if using
> > vmap() to map the very memory, so that we don't need to use such "copy_to_user" to transfer the data
> > from kernel to user-space, we can use memcpy() instead. This shall be more faster, but I had no
> > relevant data to prove it..
> >
> > So I'm not going to insist on it, it ups to you.
> >
> > D. Wythe
^ permalink raw reply
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: John Ousterhout @ 2024-12-09 16:53 UTC (permalink / raw)
To: D. Wythe; +Cc: netdev, linux-api
In-Reply-To: <b4bf68f1-189c-4685-8e1a-d8cbf60c1120@linux.alibaba.com>
Thanks for the additional information; I'll put this on my list of
things to consider for performance optimization.
-John-
On Sun, Dec 8, 2024 at 10:56 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
>
>
>
> On 12/6/24 3:49 AM, John Ousterhout wrote:
> > On Sun, Dec 1, 2024 at 7:51 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> >>> +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
> >>> + unsigned int optlen)
> >>> +{
> >>> + struct homa_sock *hsk = homa_sk(sk);
> >>> + struct homa_set_buf_args args;
> >>> + int ret;
> >>> +
> >>> + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
> >>> + optlen != sizeof(struct homa_set_buf_args))
> >>> + return -EINVAL;
> >>
> >> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
> >> implemented in getsockopt.
> >
> > I have changed it to HOMA_RCVBUF (and renamed struct homa_set_buf_args
> > to struct homa_rcvbuf_args). I also implemented getsockopt for
> > HOMA_RCVBUF.
> >
> >>> +
> >>> + if (copy_from_sockptr(&args, optval, optlen))
> >>> + return -EFAULT;
> >>> +
> >>> + /* Do a trivial test to make sure we can at least write the first
> >>> + * page of the region.
> >>> + */
> >>> + if (copy_to_user((__force void __user *)args.start, &args, sizeof(args)))
> >>> + return -EFAULT;
> >>
> >> To share buffer between kernel and userspace, maybe you should refer to the implementation of
> >> io_pin_pbuf_ring()
> >
> > I'm not sure what you mean here. Are you suggesting that I look at the
> > code of io_pin_pbuf_ring to make sure I've done everything needed to
> > share buffers? I don't believe that Homa needs to do anything special
> > (e.g. it doesn't need to pin the user's buffers); it just saves the
> > address and makes copy_to_user calls later when needed (and these
> > calls are all done at syscall level in the context of the
> > application). Is there something I'm missing?
> >
>
> I just thought that since the received buffer is shared between kernel and user-space, if using
> vmap() to map the very memory, so that we don't need to use such "copy_to_user" to transfer the data
> from kernel to user-space, we can use memcpy() instead. This shall be more faster, but I had no
> relevant data to prove it..
>
> So I'm not going to insist on it, it ups to you.
>
> D. Wythe
^ permalink raw reply
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: D. Wythe @ 2024-12-09 6:56 UTC (permalink / raw)
To: John Ousterhout; +Cc: netdev, linux-api
In-Reply-To: <CAGXJAmw4tULA02TLXBPa8Lv5cXD1Oe+1ajTWrM2x9=byMTy4jA@mail.gmail.com>
On 12/6/24 3:49 AM, John Ousterhout wrote:
> On Sun, Dec 1, 2024 at 7:51 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
>>> +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
>>> + unsigned int optlen)
>>> +{
>>> + struct homa_sock *hsk = homa_sk(sk);
>>> + struct homa_set_buf_args args;
>>> + int ret;
>>> +
>>> + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
>>> + optlen != sizeof(struct homa_set_buf_args))
>>> + return -EINVAL;
>>
>> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
>> implemented in getsockopt.
>
> I have changed it to HOMA_RCVBUF (and renamed struct homa_set_buf_args
> to struct homa_rcvbuf_args). I also implemented getsockopt for
> HOMA_RCVBUF.
>
>>> +
>>> + if (copy_from_sockptr(&args, optval, optlen))
>>> + return -EFAULT;
>>> +
>>> + /* Do a trivial test to make sure we can at least write the first
>>> + * page of the region.
>>> + */
>>> + if (copy_to_user((__force void __user *)args.start, &args, sizeof(args)))
>>> + return -EFAULT;
>>
>> To share buffer between kernel and userspace, maybe you should refer to the implementation of
>> io_pin_pbuf_ring()
>
> I'm not sure what you mean here. Are you suggesting that I look at the
> code of io_pin_pbuf_ring to make sure I've done everything needed to
> share buffers? I don't believe that Homa needs to do anything special
> (e.g. it doesn't need to pin the user's buffers); it just saves the
> address and makes copy_to_user calls later when needed (and these
> calls are all done at syscall level in the context of the
> application). Is there something I'm missing?
>
I just thought that since the received buffer is shared between kernel and user-space, if using
vmap() to map the very memory, so that we don't need to use such "copy_to_user" to transfer the data
from kernel to user-space, we can use memcpy() instead. This shall be more faster, but I had no
relevant data to prove it..
So I'm not going to insist on it, it ups to you.
D. Wythe
^ permalink raw reply
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: D. Wythe @ 2024-12-09 6:45 UTC (permalink / raw)
To: Andrew Lunn; +Cc: John Ousterhout, netdev, linux-api
In-Reply-To: <fd5cf76f-48c3-4d73-9609-c13ccb622382@lunn.ch>
On 12/3/24 9:51 AM, Andrew Lunn wrote:
> On Mon, Dec 02, 2024 at 11:51:48AM +0800, D. Wythe wrote:
>>> +/**
>>> + * homa_setsockopt() - Implements the getsockopt system call for Homa sockets.
>>> + * @sk: Socket on which the system call was invoked.
>>> + * @level: Level at which the operation should be handled; will always
>>> + * be IPPROTO_HOMA.
>>> + * @optname: Identifies a particular setsockopt operation.
>>> + * @optval: Address in user space of information about the option.
>>> + * @optlen: Number of bytes of data at @optval.
>>> + * Return: 0 on success, otherwise a negative errno.
>>> + */
>>> +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
>>> + unsigned int optlen)
>>> +{
>>> + struct homa_sock *hsk = homa_sk(sk);
>>> + struct homa_set_buf_args args;
>>> + int ret;
>>> +
>>> + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
>>> + optlen != sizeof(struct homa_set_buf_args))
>>> + return -EINVAL;
>>
>> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
>> implemented in getsockopt.
>
> Please trim the text when replying to just the needed context. With
> pages and pages of useless quoted text it is easy to overlook your
> comments.
>
> Andrew
Got it. Thanks for advice.
D. Wythe
^ permalink raw reply
* Re: [PATCH v6 00/15] integrity: Introduce the Integrity Digest Cache
From: Roberto Sassu @ 2024-12-06 15:26 UTC (permalink / raw)
To: Eric Snowberg
Cc: Mimi Zohar, Dmitry Kasatkin, corbet@lwn.net, mcgrof@kernel.org,
petr.pavlu@suse.com, samitolvanen@google.com,
da.gomez@samsung.com, Andrew Morton, paul@paul-moore.com,
jmorris@namei.org, serge@hallyn.com, shuah@kernel.org,
mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com,
linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
linux-modules@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kselftest@vger.kernel.org, wufan@linux.microsoft.com,
pbrobinson@gmail.com, zbyszek@in.waw.pl, hch@lst.de,
mjg59@srcf.ucam.org, pmatilai@redhat.com, jannh@google.com,
dhowells@redhat.com, jikos@kernel.org, mkoutny@suse.com,
ppavlu@suse.com, petr.vorel@gmail.com, mzerqung@0pointer.de,
kgold@linux.ibm.com, Roberto Sassu
In-Reply-To: <C5C46D67-F45B-406A-AD84-8CC8285E40C3@oracle.com>
On Fri, 2024-12-06 at 15:15 +0000, Eric Snowberg wrote:
>
> > On Dec 6, 2024, at 3:06 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> >
> > On Thu, 2024-12-05 at 19:41 +0000, Eric Snowberg wrote:
> > >
> > > > On Dec 5, 2024, at 9:16 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > > >
> > > > On Thu, 2024-12-05 at 09:53 +0100, Roberto Sassu wrote:
> > > > > On Thu, 2024-12-05 at 00:57 +0000, Eric Snowberg wrote:
> > > > > >
> > > > > > > On Dec 4, 2024, at 3:44 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > > > > > >
> > > > > > > On Tue, 2024-12-03 at 20:06 +0000, Eric Snowberg wrote:
> > > > > > > >
> > > > > > > > > On Nov 26, 2024, at 3:41 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, 2024-11-26 at 00:13 +0000, Eric Snowberg wrote:
> > > > > > > > > >
> > > > > > > > > > > On Nov 19, 2024, at 3:49 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > > > > > > >
> > > > > > > > > > > The Integrity Digest Cache can also help IMA for appraisal. IMA can simply
> > > > > > > > > > > lookup the calculated digest of an accessed file in the list of digests
> > > > > > > > > > > extracted from package headers, after verifying the header signature. It is
> > > > > > > > > > > sufficient to verify only one signature for all files in the package, as
> > > > > > > > > > > opposed to verifying a signature for each file.
> > > > > > > > > >
> > > > > > > > > > Is there a way to maintain integrity over time? Today if a CVE is discovered
> > > > > > > > > > in a signed program, the program hash can be added to the blacklist keyring.
> > > > > > > > > > Later if IMA appraisal is used, the signature validation will fail just for that
> > > > > > > > > > program. With the Integrity Digest Cache, is there a way to do this?
> > > > > > > > >
> > > > > > > > > As far as I can see, the ima_check_blacklist() call is before
> > > > > > > > > ima_appraise_measurement(). If it fails, appraisal with the Integrity
> > > > > > > > > Digest Cache will not be done.
> > > > > > > >
> > > > > > > >
> > > > > > > > It is good the program hash would be checked beforehand and fail if it is
> > > > > > > > contained on the list.
> > > > > > > >
> > > > > > > > The .ima keyring may contain many keys. If one of the keys was later
> > > > > > > > revoked and added to the .blacklist, wouldn't this be missed? It would
> > > > > > > > be caught during signature validation when the file is later appraised, but
> > > > > > > > now this step isn't taking place. Correct?
> > > > > > >
> > > > > > > For files included in the digest lists, yes, there won't be detection
> > > > > > > of later revocation of a key. However, it will still work at package
> > > > > > > level/digest list level, since they are still appraised with a
> > > > > > > signature.
> > > > > > >
> > > > > > > We can add a mechanism (if it does not already exist) to invalidate the
> > > > > > > integrity status based on key revocation, which can be propagated to
> > > > > > > files verified with the affected digest lists.
> > > > > > >
> > > > > > > > With IMA appraisal, it is easy to maintain authenticity but challenging to
> > > > > > > > maintain integrity over time. In user-space there are constantly new CVEs.
> > > > > > > > To maintain integrity over time, either keys need to be rotated in the .ima
> > > > > > > > keyring or program hashes need to be frequently added to the .blacklist.
> > > > > > > > If neither is done, for an end-user on a distro, IMA-appraisal basically
> > > > > > > > guarantees authenticity.
> > > > > > > >
> > > > > > > > While I understand the intent of the series is to increase performance,
> > > > > > > > have you considered using this to give the end-user the ability to maintain
> > > > > > > > integrity of their system? What I mean is, instead of trying to import anything
> > > > > > > > from an RPM, just have the end-user provide this information in some format
> > > > > > > > to the Digest Cache. User-space tools could be built to collect and format
> > > > > > >
> > > > > > > This is already possible, digest-cache-tools
> > > > > > > (https://github.com/linux-integrity/digest-cache-tools) already allow
> > > > > > > to create a digest list with the file a user wants.
> > > > > > >
> > > > > > > But in this case, the user is vouching for having taken the correct
> > > > > > > measure of the file at the time it was added to the digest list. This
> > > > > > > would be instead automatically guaranteed by RPMs or other packages
> > > > > > > shipped with Linux distributions.
> > > > > > >
> > > > > > > To mitigate the concerns of CVEs, we can probably implement a rollback
> > > > > > > prevention mechanism, which would not allow to load a previous version
> > > > > > > of a digest list.
> > > > > >
> > > > > > IMHO, pursuing this with the end-user being in control of what is contained
> > > > > > within the Digest Cache vs what is contained in a distro would provide more
> > > > > > value. Allowing the end-user to easily update their Digest Cache in some way
> > > > > > without having to do any type of revocation for both old and vulnerable
> > > > > > applications with CVEs would be very beneficial.
> > > > >
> > > > > Yes, deleting the digest list would invalidate any integrity result
> > > > > done with that digest list.
> > > > >
> > > > > I developed also an rpm plugin that synchronizes the digest lists with
> > > > > installed software. Old vulnerable software cannot be verified anymore
> > > > > with the Integrity Digest Cache, since the rpm plugin deletes the old
> > > > > software digest lists.
> > > > >
> > > > > https://github.com/linux-integrity/digest-cache-tools/blob/main/rpm-plugin/digest_cache.c
> > > > >
> > > > > The good thing is that the Integrity Digest Cache can be easily
> > > > > controlled with filesystem operations (it works similarly to security
> > > > > blobs attached to kernel objects, like inodes and file descriptors).
> > > > >
> > > > > As soon as something changes (e.g. digest list written, link to the
> > > > > digest lists), this triggers a reset in the Integrity Digest Cache, so
> > > > > digest lists and files need to be verified again. Deleting the digest
> > > > > list causes the in-kernel digest cache to be wiped away too (when the
> > > > > reference count reaches zero).
> > > > >
> > > > > > Is there a belief the Digest Cache would be used without signed kernel
> > > > > > modules? Is the performance gain worth changing how kernel modules
> > > > > > get loaded at boot? Couldn't this part just be dropped for easier acceptance?
> > > > > > Integrity is already maintained with the current model of appended signatures.
> > > > >
> > > > > I don't like making exceptions in the design, and I recently realized
> > > > > that it should not be task of the users of the Integrity Digest Cache
> > > > > to limit themselves.
> > > >
> > > > Forgot to mention that your use case is possible. The usage of the
> > > > Integrity Digest Cache must be explicitly enabled in the IMA policy. It
> > > > will be used if the matching rule has 'digest_cache=data' (its foreseen
> > > > to be used also for metadata).
> > >
> > > I see a lot of benefit if metadata integrity could be maintained, but in the
> > > current form of this series, I don't think that is possible. The Digest Cache
> > > doesn't contain or enforce the file path, which would be necessary to
> > > maintain integrity. Here is an example of why it would be needed, say
> > > you have two applications that need a configuration file to start. The first
> > > application has an empty file where no configuration options are currently
> > > defined. Now there is a hash for an empty file in the Digest Cache. The
> > > second application can be started with an empty configuration file, however
> > > the end-user has added some options to it. If the configuration file for the
> > > second application is replaced with an empty file, it will not be detected,
> > > since the Digest Cache would see the empty file hash in its cache.
> >
> > I was thinking more to store in the digest cache digests of metadata
> > (including for example the expected SELinux label), that EVM can
> > lookup.
> >
> > In that way, the problem you foresee cannot happen: if you replace the
> > file belonging to app2_t with the one belonging to app1_t, SELinux
> > would deny the permission to access; if you change the SELinux label of
> > the file, EVM will deny the access.
>
> If two different applications have config files in /etc, wouldn't both files
> have the same SELinux label?
Likely, unless there is an application-specific policy.
> > You can still go back to the initial state, for that a rollback
> > prevention mechanism is needed (maybe EVM can remove the digest of the
> > initial state from the digest cache when it sees an update?).
> >
> > In general, the Integrity Digest Cache should be considered as an
> > alternative mechanism to validate immutable files, or the initial state
> > of mutable files. For mutable files, EVM HMAC will protect further
> > updates.
>
> In the example above, from a distro standpoint, most files contained in /etc
> are viewed as being mutable. However an end-user that wants to maintain
> integrity on their system wouldn't view it that way. They don't want config
> changes they have made to be backed out. In the current form they would
> view this series as an Authenticity Digest Cache. I'm just trying to show that
> this could be a lot more valuable to the end-user if some things were changed.
I agree, I think the current patch set contains the minimum necessary,
and it can grow depending on use cases/requirements from the community.
Thanks
Roberto
^ permalink raw reply
* Re: [PATCH v6 00/15] integrity: Introduce the Integrity Digest Cache
From: Eric Snowberg @ 2024-12-06 15:15 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Dmitry Kasatkin, corbet@lwn.net, mcgrof@kernel.org,
petr.pavlu@suse.com, samitolvanen@google.com,
da.gomez@samsung.com, Andrew Morton, paul@paul-moore.com,
jmorris@namei.org, serge@hallyn.com, shuah@kernel.org,
mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com,
linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
linux-modules@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kselftest@vger.kernel.org, wufan@linux.microsoft.com,
pbrobinson@gmail.com, zbyszek@in.waw.pl, hch@lst.de,
mjg59@srcf.ucam.org, pmatilai@redhat.com, jannh@google.com,
dhowells@redhat.com, jikos@kernel.org, mkoutny@suse.com,
ppavlu@suse.com, petr.vorel@gmail.com, mzerqung@0pointer.de,
kgold@linux.ibm.com, Roberto Sassu
In-Reply-To: <d7eb63db3ee2c04fc92efe30dc9c1eadabd3f61a.camel@huaweicloud.com>
> On Dec 6, 2024, at 3:06 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>
> On Thu, 2024-12-05 at 19:41 +0000, Eric Snowberg wrote:
>>
>>> On Dec 5, 2024, at 9:16 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>>>
>>> On Thu, 2024-12-05 at 09:53 +0100, Roberto Sassu wrote:
>>>> On Thu, 2024-12-05 at 00:57 +0000, Eric Snowberg wrote:
>>>>>
>>>>>> On Dec 4, 2024, at 3:44 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>>>>>>
>>>>>> On Tue, 2024-12-03 at 20:06 +0000, Eric Snowberg wrote:
>>>>>>>
>>>>>>>> On Nov 26, 2024, at 3:41 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>>>>>>>>
>>>>>>>> On Tue, 2024-11-26 at 00:13 +0000, Eric Snowberg wrote:
>>>>>>>>>
>>>>>>>>>> On Nov 19, 2024, at 3:49 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>>>>>>>>>>
>>>>>>>>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>>>>>>>>
>>>>>>>>>> The Integrity Digest Cache can also help IMA for appraisal. IMA can simply
>>>>>>>>>> lookup the calculated digest of an accessed file in the list of digests
>>>>>>>>>> extracted from package headers, after verifying the header signature. It is
>>>>>>>>>> sufficient to verify only one signature for all files in the package, as
>>>>>>>>>> opposed to verifying a signature for each file.
>>>>>>>>>
>>>>>>>>> Is there a way to maintain integrity over time? Today if a CVE is discovered
>>>>>>>>> in a signed program, the program hash can be added to the blacklist keyring.
>>>>>>>>> Later if IMA appraisal is used, the signature validation will fail just for that
>>>>>>>>> program. With the Integrity Digest Cache, is there a way to do this?
>>>>>>>>
>>>>>>>> As far as I can see, the ima_check_blacklist() call is before
>>>>>>>> ima_appraise_measurement(). If it fails, appraisal with the Integrity
>>>>>>>> Digest Cache will not be done.
>>>>>>>
>>>>>>>
>>>>>>> It is good the program hash would be checked beforehand and fail if it is
>>>>>>> contained on the list.
>>>>>>>
>>>>>>> The .ima keyring may contain many keys. If one of the keys was later
>>>>>>> revoked and added to the .blacklist, wouldn't this be missed? It would
>>>>>>> be caught during signature validation when the file is later appraised, but
>>>>>>> now this step isn't taking place. Correct?
>>>>>>
>>>>>> For files included in the digest lists, yes, there won't be detection
>>>>>> of later revocation of a key. However, it will still work at package
>>>>>> level/digest list level, since they are still appraised with a
>>>>>> signature.
>>>>>>
>>>>>> We can add a mechanism (if it does not already exist) to invalidate the
>>>>>> integrity status based on key revocation, which can be propagated to
>>>>>> files verified with the affected digest lists.
>>>>>>
>>>>>>> With IMA appraisal, it is easy to maintain authenticity but challenging to
>>>>>>> maintain integrity over time. In user-space there are constantly new CVEs.
>>>>>>> To maintain integrity over time, either keys need to be rotated in the .ima
>>>>>>> keyring or program hashes need to be frequently added to the .blacklist.
>>>>>>> If neither is done, for an end-user on a distro, IMA-appraisal basically
>>>>>>> guarantees authenticity.
>>>>>>>
>>>>>>> While I understand the intent of the series is to increase performance,
>>>>>>> have you considered using this to give the end-user the ability to maintain
>>>>>>> integrity of their system? What I mean is, instead of trying to import anything
>>>>>>> from an RPM, just have the end-user provide this information in some format
>>>>>>> to the Digest Cache. User-space tools could be built to collect and format
>>>>>>
>>>>>> This is already possible, digest-cache-tools
>>>>>> (https://github.com/linux-integrity/digest-cache-tools) already allow
>>>>>> to create a digest list with the file a user wants.
>>>>>>
>>>>>> But in this case, the user is vouching for having taken the correct
>>>>>> measure of the file at the time it was added to the digest list. This
>>>>>> would be instead automatically guaranteed by RPMs or other packages
>>>>>> shipped with Linux distributions.
>>>>>>
>>>>>> To mitigate the concerns of CVEs, we can probably implement a rollback
>>>>>> prevention mechanism, which would not allow to load a previous version
>>>>>> of a digest list.
>>>>>
>>>>> IMHO, pursuing this with the end-user being in control of what is contained
>>>>> within the Digest Cache vs what is contained in a distro would provide more
>>>>> value. Allowing the end-user to easily update their Digest Cache in some way
>>>>> without having to do any type of revocation for both old and vulnerable
>>>>> applications with CVEs would be very beneficial.
>>>>
>>>> Yes, deleting the digest list would invalidate any integrity result
>>>> done with that digest list.
>>>>
>>>> I developed also an rpm plugin that synchronizes the digest lists with
>>>> installed software. Old vulnerable software cannot be verified anymore
>>>> with the Integrity Digest Cache, since the rpm plugin deletes the old
>>>> software digest lists.
>>>>
>>>> https://github.com/linux-integrity/digest-cache-tools/blob/main/rpm-plugin/digest_cache.c
>>>>
>>>> The good thing is that the Integrity Digest Cache can be easily
>>>> controlled with filesystem operations (it works similarly to security
>>>> blobs attached to kernel objects, like inodes and file descriptors).
>>>>
>>>> As soon as something changes (e.g. digest list written, link to the
>>>> digest lists), this triggers a reset in the Integrity Digest Cache, so
>>>> digest lists and files need to be verified again. Deleting the digest
>>>> list causes the in-kernel digest cache to be wiped away too (when the
>>>> reference count reaches zero).
>>>>
>>>>> Is there a belief the Digest Cache would be used without signed kernel
>>>>> modules? Is the performance gain worth changing how kernel modules
>>>>> get loaded at boot? Couldn't this part just be dropped for easier acceptance?
>>>>> Integrity is already maintained with the current model of appended signatures.
>>>>
>>>> I don't like making exceptions in the design, and I recently realized
>>>> that it should not be task of the users of the Integrity Digest Cache
>>>> to limit themselves.
>>>
>>> Forgot to mention that your use case is possible. The usage of the
>>> Integrity Digest Cache must be explicitly enabled in the IMA policy. It
>>> will be used if the matching rule has 'digest_cache=data' (its foreseen
>>> to be used also for metadata).
>>
>> I see a lot of benefit if metadata integrity could be maintained, but in the
>> current form of this series, I don't think that is possible. The Digest Cache
>> doesn't contain or enforce the file path, which would be necessary to
>> maintain integrity. Here is an example of why it would be needed, say
>> you have two applications that need a configuration file to start. The first
>> application has an empty file where no configuration options are currently
>> defined. Now there is a hash for an empty file in the Digest Cache. The
>> second application can be started with an empty configuration file, however
>> the end-user has added some options to it. If the configuration file for the
>> second application is replaced with an empty file, it will not be detected,
>> since the Digest Cache would see the empty file hash in its cache.
>
> I was thinking more to store in the digest cache digests of metadata
> (including for example the expected SELinux label), that EVM can
> lookup.
>
> In that way, the problem you foresee cannot happen: if you replace the
> file belonging to app2_t with the one belonging to app1_t, SELinux
> would deny the permission to access; if you change the SELinux label of
> the file, EVM will deny the access.
If two different applications have config files in /etc, wouldn't both files
have the same SELinux label?
> You can still go back to the initial state, for that a rollback
> prevention mechanism is needed (maybe EVM can remove the digest of the
> initial state from the digest cache when it sees an update?).
>
> In general, the Integrity Digest Cache should be considered as an
> alternative mechanism to validate immutable files, or the initial state
> of mutable files. For mutable files, EVM HMAC will protect further
> updates.
In the example above, from a distro standpoint, most files contained in /etc
are viewed as being mutable. However an end-user that wants to maintain
integrity on their system wouldn't view it that way. They don't want config
changes they have made to be backed out. In the current form they would
view this series as an Authenticity Digest Cache. I'm just trying to show that
this could be a lot more valuable to the end-user if some things were changed.
^ permalink raw reply
* Re: [PATCH v6 00/15] integrity: Introduce the Integrity Digest Cache
From: Roberto Sassu @ 2024-12-06 10:06 UTC (permalink / raw)
To: Eric Snowberg
Cc: Mimi Zohar, Dmitry Kasatkin, corbet@lwn.net, mcgrof@kernel.org,
petr.pavlu@suse.com, samitolvanen@google.com,
da.gomez@samsung.com, Andrew Morton, paul@paul-moore.com,
jmorris@namei.org, serge@hallyn.com, shuah@kernel.org,
mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com,
linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
linux-modules@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kselftest@vger.kernel.org, wufan@linux.microsoft.com,
pbrobinson@gmail.com, zbyszek@in.waw.pl, hch@lst.de,
mjg59@srcf.ucam.org, pmatilai@redhat.com, jannh@google.com,
dhowells@redhat.com, jikos@kernel.org, mkoutny@suse.com,
ppavlu@suse.com, petr.vorel@gmail.com, mzerqung@0pointer.de,
kgold@linux.ibm.com, Roberto Sassu
In-Reply-To: <F9EE3709-2A90-4E9A-8531-44EC0F9C240C@oracle.com>
On Thu, 2024-12-05 at 19:41 +0000, Eric Snowberg wrote:
>
> > On Dec 5, 2024, at 9:16 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> >
> > On Thu, 2024-12-05 at 09:53 +0100, Roberto Sassu wrote:
> > > On Thu, 2024-12-05 at 00:57 +0000, Eric Snowberg wrote:
> > > >
> > > > > On Dec 4, 2024, at 3:44 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > > > >
> > > > > On Tue, 2024-12-03 at 20:06 +0000, Eric Snowberg wrote:
> > > > > >
> > > > > > > On Nov 26, 2024, at 3:41 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > > > > > >
> > > > > > > On Tue, 2024-11-26 at 00:13 +0000, Eric Snowberg wrote:
> > > > > > > >
> > > > > > > > > On Nov 19, 2024, at 3:49 AM, Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > >
> > > > > > > > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > > > > >
> > > > > > > > > The Integrity Digest Cache can also help IMA for appraisal. IMA can simply
> > > > > > > > > lookup the calculated digest of an accessed file in the list of digests
> > > > > > > > > extracted from package headers, after verifying the header signature. It is
> > > > > > > > > sufficient to verify only one signature for all files in the package, as
> > > > > > > > > opposed to verifying a signature for each file.
> > > > > > > >
> > > > > > > > Is there a way to maintain integrity over time? Today if a CVE is discovered
> > > > > > > > in a signed program, the program hash can be added to the blacklist keyring.
> > > > > > > > Later if IMA appraisal is used, the signature validation will fail just for that
> > > > > > > > program. With the Integrity Digest Cache, is there a way to do this?
> > > > > > >
> > > > > > > As far as I can see, the ima_check_blacklist() call is before
> > > > > > > ima_appraise_measurement(). If it fails, appraisal with the Integrity
> > > > > > > Digest Cache will not be done.
> > > > > >
> > > > > >
> > > > > > It is good the program hash would be checked beforehand and fail if it is
> > > > > > contained on the list.
> > > > > >
> > > > > > The .ima keyring may contain many keys. If one of the keys was later
> > > > > > revoked and added to the .blacklist, wouldn't this be missed? It would
> > > > > > be caught during signature validation when the file is later appraised, but
> > > > > > now this step isn't taking place. Correct?
> > > > >
> > > > > For files included in the digest lists, yes, there won't be detection
> > > > > of later revocation of a key. However, it will still work at package
> > > > > level/digest list level, since they are still appraised with a
> > > > > signature.
> > > > >
> > > > > We can add a mechanism (if it does not already exist) to invalidate the
> > > > > integrity status based on key revocation, which can be propagated to
> > > > > files verified with the affected digest lists.
> > > > >
> > > > > > With IMA appraisal, it is easy to maintain authenticity but challenging to
> > > > > > maintain integrity over time. In user-space there are constantly new CVEs.
> > > > > > To maintain integrity over time, either keys need to be rotated in the .ima
> > > > > > keyring or program hashes need to be frequently added to the .blacklist.
> > > > > > If neither is done, for an end-user on a distro, IMA-appraisal basically
> > > > > > guarantees authenticity.
> > > > > >
> > > > > > While I understand the intent of the series is to increase performance,
> > > > > > have you considered using this to give the end-user the ability to maintain
> > > > > > integrity of their system? What I mean is, instead of trying to import anything
> > > > > > from an RPM, just have the end-user provide this information in some format
> > > > > > to the Digest Cache. User-space tools could be built to collect and format
> > > > >
> > > > > This is already possible, digest-cache-tools
> > > > > (https://github.com/linux-integrity/digest-cache-tools) already allow
> > > > > to create a digest list with the file a user wants.
> > > > >
> > > > > But in this case, the user is vouching for having taken the correct
> > > > > measure of the file at the time it was added to the digest list. This
> > > > > would be instead automatically guaranteed by RPMs or other packages
> > > > > shipped with Linux distributions.
> > > > >
> > > > > To mitigate the concerns of CVEs, we can probably implement a rollback
> > > > > prevention mechanism, which would not allow to load a previous version
> > > > > of a digest list.
> > > >
> > > > IMHO, pursuing this with the end-user being in control of what is contained
> > > > within the Digest Cache vs what is contained in a distro would provide more
> > > > value. Allowing the end-user to easily update their Digest Cache in some way
> > > > without having to do any type of revocation for both old and vulnerable
> > > > applications with CVEs would be very beneficial.
> > >
> > > Yes, deleting the digest list would invalidate any integrity result
> > > done with that digest list.
> > >
> > > I developed also an rpm plugin that synchronizes the digest lists with
> > > installed software. Old vulnerable software cannot be verified anymore
> > > with the Integrity Digest Cache, since the rpm plugin deletes the old
> > > software digest lists.
> > >
> > > https://github.com/linux-integrity/digest-cache-tools/blob/main/rpm-plugin/digest_cache.c
> > >
> > > The good thing is that the Integrity Digest Cache can be easily
> > > controlled with filesystem operations (it works similarly to security
> > > blobs attached to kernel objects, like inodes and file descriptors).
> > >
> > > As soon as something changes (e.g. digest list written, link to the
> > > digest lists), this triggers a reset in the Integrity Digest Cache, so
> > > digest lists and files need to be verified again. Deleting the digest
> > > list causes the in-kernel digest cache to be wiped away too (when the
> > > reference count reaches zero).
> > >
> > > > Is there a belief the Digest Cache would be used without signed kernel
> > > > modules? Is the performance gain worth changing how kernel modules
> > > > get loaded at boot? Couldn't this part just be dropped for easier acceptance?
> > > > Integrity is already maintained with the current model of appended signatures.
> > >
> > > I don't like making exceptions in the design, and I recently realized
> > > that it should not be task of the users of the Integrity Digest Cache
> > > to limit themselves.
> >
> > Forgot to mention that your use case is possible. The usage of the
> > Integrity Digest Cache must be explicitly enabled in the IMA policy. It
> > will be used if the matching rule has 'digest_cache=data' (its foreseen
> > to be used also for metadata).
>
> I see a lot of benefit if metadata integrity could be maintained, but in the
> current form of this series, I don't think that is possible. The Digest Cache
> doesn't contain or enforce the file path, which would be necessary to
> maintain integrity. Here is an example of why it would be needed, say
> you have two applications that need a configuration file to start. The first
> application has an empty file where no configuration options are currently
> defined. Now there is a hash for an empty file in the Digest Cache. The
> second application can be started with an empty configuration file, however
> the end-user has added some options to it. If the configuration file for the
> second application is replaced with an empty file, it will not be detected,
> since the Digest Cache would see the empty file hash in its cache.
I was thinking more to store in the digest cache digests of metadata
(including for example the expected SELinux label), that EVM can
lookup.
In that way, the problem you foresee cannot happen: if you replace the
file belonging to app2_t with the one belonging to app1_t, SELinux
would deny the permission to access; if you change the SELinux label of
the file, EVM will deny the access.
You can still go back to the initial state, for that a rollback
prevention mechanism is needed (maybe EVM can remove the digest of the
initial state from the digest cache when it sees an update?).
In general, the Integrity Digest Cache should be considered as an
alternative mechanism to validate immutable files, or the initial state
of mutable files. For mutable files, EVM HMAC will protect further
updates.
Roberto
> > For kernel modules, it is sufficient to not provide that keyword for
> > the MODULE_CHECK hook.
> >
> > However, there is the possibility that you lose another advantage of
> > the Integrity Digest Cache, the predictability of the IMA PCR. By not
> > using digest caches, there is the risk that the IMA PCR will be
> > unstable, due to loading kernel modules in a different order at each
> > boot.
>
> Understood, my recommendation was based on trying to narrow the series
> to help try to get something like this adopted quicker.
>
^ permalink raw reply
* Re: [PATCH net-next v2 11/12] net: homa: create homa_plumbing.c homa_utils.c
From: John Ousterhout @ 2024-12-05 19:49 UTC (permalink / raw)
To: D. Wythe; +Cc: netdev, linux-api
In-Reply-To: <f79a70fd-35a4-4d0d-b239-daa4ab652880@linux.alibaba.com>
On Sun, Dec 1, 2024 at 7:51 PM D. Wythe <alibuda@linux.alibaba.com> wrote:
> > +int homa_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
> > + unsigned int optlen)
> > +{
> > + struct homa_sock *hsk = homa_sk(sk);
> > + struct homa_set_buf_args args;
> > + int ret;
> > +
> > + if (level != IPPROTO_HOMA || optname != SO_HOMA_SET_BUF ||
> > + optlen != sizeof(struct homa_set_buf_args))
> > + return -EINVAL;
>
> SO_HOMA_SET_BUF is a bit odd here, maybe HOMA_RCVBUF ? which also can be
> implemented in getsockopt.
I have changed it to HOMA_RCVBUF (and renamed struct homa_set_buf_args
to struct homa_rcvbuf_args). I also implemented getsockopt for
HOMA_RCVBUF.
> > +
> > + if (copy_from_sockptr(&args, optval, optlen))
> > + return -EFAULT;
> > +
> > + /* Do a trivial test to make sure we can at least write the first
> > + * page of the region.
> > + */
> > + if (copy_to_user((__force void __user *)args.start, &args, sizeof(args)))
> > + return -EFAULT;
>
> To share buffer between kernel and userspace, maybe you should refer to the implementation of
> io_pin_pbuf_ring()
I'm not sure what you mean here. Are you suggesting that I look at the
code of io_pin_pbuf_ring to make sure I've done everything needed to
share buffers? I don't believe that Homa needs to do anything special
(e.g. it doesn't need to pin the user's buffers); it just saves the
address and makes copy_to_user calls later when needed (and these
calls are all done at syscall level in the context of the
application). Is there something I'm missing?
> > +
> > + homa_sock_lock(hsk, "homa_setsockopt SO_HOMA_SET_BUF");
> > + ret = homa_pool_init(hsk, (__force void __user *)args.start, args.length);
> > + homa_sock_unlock(hsk);
>
> It seems that if the option was not set, poll->hsk will not be initialized
> and then if recvmsg is called, a panic will be triggered.
I fixed homa_pool_check_waiting to properly handle empty pools.
> > +/**
> > + * homa_sendmsg() - Send a request or response message on a Homa socket.
> > + * @sk: Socket on which the system call was invoked.
> > + * @msg: Structure describing the message to send; the msg_control
> > + * field points to additional information.
> > + * @length: Number of bytes of the message.
> > + * Return: 0 on success, otherwise a negative errno.
> > + */
> > +int homa_sendmsg(struct sock *sk, struct msghdr *msg, size_t length)
> > +{
> > + struct homa_sock *hsk = homa_sk(sk);
> > + struct homa_sendmsg_args args;
> > + int result = 0;
> > + struct homa_rpc *rpc = NULL;
> > + union sockaddr_in_union *addr = (union sockaddr_in_union *)msg->msg_name;
>
> msg->msg_name can be NULL.
I have added a check for this.
> > + if (addr->in6.sin6_family != sk->sk_family) {
> > + result = -EAFNOSUPPORT;
> > + goto error;
> > + }
>
> addr->sa.sa_family? Using sin6_family would be odd to me, making it seem like homa can only run with
> IPv6.
The family is in the same place for all protocols so I thought it
would be safe to reference it as in6.sin6_family. But you're right
that it looks odd, so I changed it to addr->sa.sa_family.
> > + rpc = homa_rpc_new_client(hsk, addr);
> > + if (IS_ERR(rpc)) {
> > + result = PTR_ERR(rpc);
> > + rpc = NULL;
> > + goto error;
> > + }
> > + rpc->completion_cookie = args.completion_cookie;
> > + result = homa_message_out_fill(rpc, &msg->msg_iter, 1);
> > + if (result)
> > + goto error;
> > + args.id = rpc->id;
> > + homa_rpc_unlock(rpc);
>
> Strongly recommend that adding comments with all the unlock part to indicate where it was
> locked from.
I went through the code and added comments in places where the locking
isn't relatively obvious (e.g. it happens through methods without
"lock" in their name, such as homa_rpc_new_client). I also renamed
homa_try_bucket_lock to homa_try_rpc_lock, which will clarify RPC
locking a bit better.
> > + canonical_dest = canonical_ipv6_addr(addr);
>
> Are you treating all addresses as IPv6 addresses just for the sake of simplicity? It's a bit odd,
> but okay to me.
Yes: Homa uses IPv6 addresses internally for both IPv4 and IPv6; this
allows Homa to work with both IPv4 and IPv6 with only a small amount
of code that is protocol-specific.
> > +int homa_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,
> > + int *addr_len)
> > +{
> > + struct homa_sock *hsk = homa_sk(sk);
> > + struct homa_recvmsg_args control;
> > + struct homa_rpc *rpc;
> > + int result;
> > +
> > + if (unlikely(!msg->msg_control)) {
> > + /* This test isn't strictly necessary, but it provides a
> > + * hook for testing kernel call times.
> > + */
> > + return -EINVAL;
> > + }
> > + if (msg->msg_controllen != sizeof(control)) {
> > + result = -EINVAL;
> > + goto done;
>
> Then you copied an uninitialized control into userspace ...
> Is goto really necessary ? Maybe just return.
Good catch; I changed it to just return. In general, I prefer to
funnel everything through a single cleanup-and-return point rather
than returning directly in some cases and jumping to a cleanup point
in others; I think that makes the code cleaner and more obvious. But,
that doesn't really work here.
>
> > + }
> > + if (unlikely(copy_from_user(&control,
> > + (__force void __user *)msg->msg_control,
> > + sizeof(control)))) {
> > + result = -EFAULT;
> > + goto done;
>
> Same as above, is goto really necessary ?
I changed it to return directly.
> > + }
> > + control.completion_cookie = 0;
> > +
> > + if (control.num_bpages > HOMA_MAX_BPAGES ||
> > + (control.flags & ~HOMA_RECVMSG_VALID_FLAGS)) {
> > + result = -EINVAL;
> > + goto done;
> > + }
> > + homa_pool_release_buffers(hsk->buffer_pool, control.num_bpages,
> > + control.bpage_offsets);
>
> homa_pool_release_buffers() quietly ignores erroneous bpage_index values passed in from the
> userspace. This behavior may obscure more complex issues in userspace, and exposure this problem
> could help users identify issues earlier.
Good point; homa_recvmsg now returns -EINVAL if there are erroneous
bpage_index values passed in.
> > + control.num_bpages = 0;
> > +
> > + rpc = homa_wait_for_message(hsk, (flags & MSG_DONTWAIT)
> > + ? (control.flags | HOMA_RECVMSG_NONBLOCKING)
> > + : control.flags, control.id);
> > + if (IS_ERR(rpc)) {
> > + /* If we get here, it means there was an error that prevented
> > + * us from finding an RPC to return. If there's an error in
> > + * the RPC itself we won't get here.
> > + */
> > + result = PTR_ERR(rpc);
> > + goto done;
> > + }
> > + result = rpc->error ? rpc->error : rpc->msgin.length;
>
> A trivial tips.
>
> result = rpc->error ?: rpc->msgin.length;
I wasn't aware that the shorter form existed. However, I find the
longer form a bit more obvious, so I'd prefer to leave that unless you
feel strongly that it should be abbreviated.
> > +
> > + /* Collect result information. */
> > + control.id = rpc->id;
> > + control.completion_cookie = rpc->completion_cookie;
> > + if (likely(rpc->msgin.length >= 0)) {
> > + control.num_bpages = rpc->msgin.num_bpages;
> > + memcpy(control.bpage_offsets, rpc->msgin.bpage_offsets,
> > + sizeof(control.bpage_offsets));
>
> A trivial tips.
>
> Although sizeof(control.bpage_offsets) and sizeof(rpc->msgin.bpage_offsets) are the same, but
> passing sizeof(rpc->msgin.bpage_offsets) would be more appropriate.
Done.
Thanks for the comments; lots of good caches there.
-John-
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox