[RFC,3/9] ntsync: Introduce NTSYNC_IOC_CREATE_SEM and NTSYNC_IOC_DELETE.
Commit Message
These correspond to the NT syscalls NtCreateSemaphore() and NtClose().
Unlike those functions, however, these ioctls do not handle object names, or
lookup of existing objects, or handle reference counting, but simply create the
underlying primitive. The user space emulator is expected to implement those
functions if they are required.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 117 ++++++++++++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 25 ++++++++
2 files changed, 142 insertions(+)
create mode 100644 include/uapi/linux/ntsync.h
Comments
On Tue, Jan 23, 2024 at 06:40:22PM -0600, Elizabeth Figura wrote:
> +static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
> +{
> + struct ntsync_sem_args __user *user_args = argp;
> + struct ntsync_sem_args args;
> + struct ntsync_obj *sem;
> + __u32 id;
> + int ret;
> +
> + if (copy_from_user(&args, argp, sizeof(args)))
> + return -EFAULT;
> +
> + if (args.count > args.max)
> + return -EINVAL;
No bounds checking on count or max?
What's the relationship between count and max? Some sort of real
documentation is needed here, the changelog needs to explain this. Or
somewhere, but as-is, this patch series is pretty unreviewable as I
can't figure out how to review it because I don't know what it wants to
do.
thanks,
greg k-h
On Tuesday, 23 January 2024 19:14:17 CST Greg Kroah-Hartman wrote:
> On Tue, Jan 23, 2024 at 06:40:22PM -0600, Elizabeth Figura wrote:
> > +static int ntsync_create_sem(struct ntsync_device *dev, void __user
> > *argp)
> > +{
> > + struct ntsync_sem_args __user *user_args = argp;
> > + struct ntsync_sem_args args;
> > + struct ntsync_obj *sem;
> > + __u32 id;
> > + int ret;
> > +
> > + if (copy_from_user(&args, argp, sizeof(args)))
> > + return -EFAULT;
> > +
> > + if (args.count > args.max)
> > + return -EINVAL;
>
> No bounds checking on count or max?
>
> What's the relationship between count and max?
Indeed, no bounds checking. The counter is just the semaphore's internal value
and has no meaning other than that.
It's basically like an EFD_SEMAPHORE, except that the maximum is configurable
rather than always being 2**64-2.
> Some sort of real
> documentation is needed here, the changelog needs to explain this. Or
> somewhere, but as-is, this patch series is pretty unreviewable as I
> can't figure out how to review it because I don't know what it wants to
> do.
There is some comprehensive documentation in the series, but for ease of
review I will try to write a basic description of the API in each relevant
patch in v2.
@@ -8,23 +8,140 @@
#include <linux/fs.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/xarray.h>
+#include <uapi/linux/ntsync.h>
#define NTSYNC_NAME "ntsync"
+enum ntsync_type {
+ NTSYNC_TYPE_SEM,
+};
+
+struct ntsync_obj {
+ struct rcu_head rhead;
+ struct kref refcount;
+
+ enum ntsync_type type;
+
+ union {
+ struct {
+ __u32 count;
+ __u32 max;
+ } sem;
+ } u;
+};
+
+struct ntsync_device {
+ struct xarray objects;
+};
+
+static void destroy_obj(struct kref *ref)
+{
+ struct ntsync_obj *obj = container_of(ref, struct ntsync_obj, refcount);
+
+ kfree_rcu(obj, rhead);
+}
+
+static void put_obj(struct ntsync_obj *obj)
+{
+ kref_put(&obj->refcount, destroy_obj);
+}
+
static int ntsync_char_open(struct inode *inode, struct file *file)
{
+ struct ntsync_device *dev;
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ xa_init_flags(&dev->objects, XA_FLAGS_ALLOC);
+
+ file->private_data = dev;
return nonseekable_open(inode, file);
}
static int ntsync_char_release(struct inode *inode, struct file *file)
{
+ struct ntsync_device *dev = file->private_data;
+ struct ntsync_obj *obj;
+ unsigned long id;
+
+ xa_for_each(&dev->objects, id, obj)
+ put_obj(obj);
+
+ xa_destroy(&dev->objects);
+
+ kfree(dev);
+
+ return 0;
+}
+
+static void init_obj(struct ntsync_obj *obj)
+{
+ kref_init(&obj->refcount);
+}
+
+static int ntsync_create_sem(struct ntsync_device *dev, void __user *argp)
+{
+ struct ntsync_sem_args __user *user_args = argp;
+ struct ntsync_sem_args args;
+ struct ntsync_obj *sem;
+ __u32 id;
+ int ret;
+
+ if (copy_from_user(&args, argp, sizeof(args)))
+ return -EFAULT;
+
+ if (args.count > args.max)
+ return -EINVAL;
+
+ sem = kzalloc(sizeof(*sem), GFP_KERNEL);
+ if (!sem)
+ return -ENOMEM;
+
+ init_obj(sem);
+ sem->type = NTSYNC_TYPE_SEM;
+ sem->u.sem.count = args.count;
+ sem->u.sem.max = args.max;
+
+ ret = xa_alloc(&dev->objects, &id, sem, xa_limit_32b, GFP_KERNEL);
+ if (ret < 0) {
+ kfree(sem);
+ return ret;
+ }
+
+ return put_user(id, &user_args->sem);
+}
+
+static int ntsync_delete(struct ntsync_device *dev, void __user *argp)
+{
+ struct ntsync_obj *obj;
+ __u32 id;
+
+ if (get_user(id, (__u32 __user *)argp))
+ return -EFAULT;
+
+ obj = xa_erase(&dev->objects, id);
+ if (!obj)
+ return -EINVAL;
+
+ put_obj(obj);
return 0;
}
static long ntsync_char_ioctl(struct file *file, unsigned int cmd,
unsigned long parm)
{
+ struct ntsync_device *dev = file->private_data;
+ void __user *argp = (void __user *)parm;
+
switch (cmd) {
+ case NTSYNC_IOC_CREATE_SEM:
+ return ntsync_create_sem(dev, argp);
+ case NTSYNC_IOC_DELETE:
+ return ntsync_delete(dev, argp);
default:
return -ENOIOCTLCMD;
}
new file mode 100644
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Kernel support for NT synchronization primitive emulation
+ *
+ * Copyright (C) 2021-2022 Elizabeth Figura
+ */
+
+#ifndef __LINUX_NTSYNC_H
+#define __LINUX_NTSYNC_H
+
+#include <linux/types.h>
+
+struct ntsync_sem_args {
+ __u32 sem;
+ __u32 count;
+ __u32 max;
+};
+
+#define NTSYNC_IOC_BASE 0xf7
+
+#define NTSYNC_IOC_CREATE_SEM _IOWR(NTSYNC_IOC_BASE, 0, \
+ struct ntsync_sem_args)
+#define NTSYNC_IOC_DELETE _IOW (NTSYNC_IOC_BASE, 1, __u32)
+
+#endif