lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sat, 27 Sep 2008 18:44:01 +0300
From:	Avi Kivity <avi@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	viro@...IV.linux.org.uk, linux-kernel@...r.kernel.org
Subject: [PATCH 2/3] ioctl: extensible ioctl dispatch

ioctls (as traditionally implemented) are very brittle; one cannot add
a field to the argument structure without breaking the ABI.  In many
cases, this is a good thing as the ABI was not designed for extension.

In other cases, however, it makes sense to design an ABI with extension
in mind.  This patch provides a new ioctl_dispatch_extensible(), which
is designed to cope with evolving interfaces:

- the ioctl number is matched only by its number and type, allowing
  size and direction to change
- buffer space is allocated for the size the kernel expects, even if
  the user passed a smaller buffer
- copying is limited by the size the kernel expects, even if the user
  passed a larger buffer
- buffer size mismatches are handled by zeroing

The ABI can account for new fields either by recognizing zero as a "missing"
value, or by implementing bitflags for feature availability.  Both an old
kernel with new userspace and a new kernel with old userspace are accomodated.

Signed-off-by: Avi Kivity <avi@...hat.com>
---
 fs/ioctl.c            |   87 +++++++++++++++++++++++++++++++++++++++++-------
 include/linux/ioctl.h |    4 ++
 2 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index f63d5ce..5a354fc 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -225,12 +225,13 @@ asmlinkage long sys_ioctl(unsigned int fd, unsigned int cmd, unsigned long arg)
  * Locates and calls ioctl handler in @handlers; if none exist, calls
  * @fallback; if that does not exist, return -ENOTTY.
  */
-long dispatch_ioctl(struct inode *inode, struct file *filp,
-		    unsigned cmd, unsigned long arg,
-		    const struct ioctl_handler *handlers,
-		    long (*fallback)(const struct ioctl_arg *arg))
+static long __dispatch_ioctl(struct inode *inode, struct file *filp,
+			     unsigned cmd, unsigned long arg,
+			     const struct ioctl_handler *handlers,
+			     long (*fallback)(const struct ioctl_arg *arg),
+			     unsigned cmd_mask)
 {
-	unsigned dir, size;
+	unsigned dir, size, usize, copysize;
 	long ret;
 	long stackbuf[IOCTL_STACK_BUF / sizeof(long)];
 	struct ioctl_arg iarg;
@@ -242,7 +243,7 @@ long dispatch_ioctl(struct inode *inode, struct file *filp,
 	 * used (and performance sensitive) items are in the front.
 	 */
 	for (; handlers->handler; ++handlers)
-		if (handlers->cmd == cmd) {
+		if ((handlers->cmd & cmd_mask) == (cmd & cmd_mask)) {
 			handler = handlers->handler;
 			break;
 		}
@@ -255,10 +256,14 @@ long dispatch_ioctl(struct inode *inode, struct file *filp,
 	iarg.inode = inode;
 	iarg.argl = arg;
 
-	size = 0;
+	size = usize = 0;
 	dir = _IOC_DIR(cmd);
-	if (dir != _IOC_NONE)
-		size = _IOC_SIZE(cmd);
+	if (dir != _IOC_NONE) {
+		size = _IOC_SIZE(handlers->cmd);
+		usize = _IOC_SIZE(cmd);
+		if (!handlers->handler)
+			size = usize;
+	}
 
 	iarg.argp = stackbuf;
 	if (size > sizeof(stackbuf)) {
@@ -266,18 +271,28 @@ long dispatch_ioctl(struct inode *inode, struct file *filp,
 		if (!iarg.argp)
 			return -ENOMEM;
 	}
-	if (dir & _IOC_WRITE)
-		if (copy_from_user(iarg.argp, (void __user *)arg, size))
+
+	if (dir & _IOC_WRITE) {
+		copysize = min(size, usize);
+		if (copy_from_user(iarg.argp, (void __user *)arg, copysize))
 			goto out_fault;
+		if (copysize < size)
+			memset(iarg.argp + copysize, 0, size - copysize);
+	}
 
 	ret = handler(&iarg);
 
 	if (ret < 0)
 		goto out;
 
-	if (dir & _IOC_READ)
-		if (copy_to_user((void __user *)arg, iarg.argp, size))
+	if (dir & _IOC_READ) {
+		copysize = min(size, usize);
+		if (copy_to_user((void __user *)arg, iarg.argp, copysize))
 			goto out_fault;
+		for (; copysize < usize; ++copysize)
+			if (put_user(0, (u8 __user *)iarg.argp + copysize))
+				goto out_fault;
+	}
 out:
 	if (iarg.argp != stackbuf)
 		kfree(iarg.argp);
@@ -288,4 +303,50 @@ out_fault:
 	ret = -EFAULT;
 	goto out;
 }
+
+/**
+ * dispatch_ioctl - dispatch to an ioctl handler based on ioctl number
+ * @inode:	inode to invoke ioctl method on
+ * @filp:	open file to invoke ioctl method on
+ * @cmd:	ioctl command to execute
+ * @arg:	command-specific argument for ioctl
+ * @handlers:	list of potential handlers for @cmd; null terminated;
+ *              place frequently used cmds first
+ * @fallback:   optional function to call if @cmd not found in @handlers
+ *
+ * Locates and calls ioctl handler in @handlers; if none exist, calls
+ * @fallback; if that does not exist, return -ENOTTY.
+ */
+long dispatch_ioctl(struct inode *inode, struct file *filp,
+		    unsigned cmd, unsigned long arg,
+		    const struct ioctl_handler *handlers,
+		    long (*fallback)(const struct ioctl_arg *arg))
+{
+	return __dispatch_ioctl(inode, filp, cmd, arg, handlers, fallback, ~0);
+}
 EXPORT_SYMBOL_GPL(dispatch_ioctl);
+
+/**
+ * dispatch_ioctl_extensible - dispatch to an ioctl handler based on ioctl
+ *                             number; adjust for differing buffer sizes
+ *                             between user and kernel
+ * @inode:	inode to invoke ioctl method on
+ * @filp:	open file to invoke ioctl method on
+ * @cmd:	ioctl command to execute
+ * @arg:	command-specific argument for ioctl
+ * @handlers:	list of potential handlers for @cmd; null terminated;
+ *              place frequently used cmds first
+ * @fallback:   optional function to call if @cmd not found in @handlers
+ *
+ * Locates and calls ioctl handler in @handlers; if none exist, calls
+ * @fallback; if that does not exist, return -ENOTTY.
+ */
+long dispatch_ioctl_extensible(struct inode *inode, struct file *filp,
+			       unsigned cmd, unsigned long arg,
+			       const struct ioctl_handler *handlers,
+			       long (*fallback)(const struct ioctl_arg *arg))
+{
+	return __dispatch_ioctl(inode, filp, cmd, arg, handlers, fallback,
+				~(_IOC_SIZEMASK << _IOC_SIZESHIFT));
+}
+EXPORT_SYMBOL_GPL(dispatch_ioctl_extensible);
diff --git a/include/linux/ioctl.h b/include/linux/ioctl.h
index 881974a..5e73fbf 100644
--- a/include/linux/ioctl.h
+++ b/include/linux/ioctl.h
@@ -33,6 +33,10 @@ long dispatch_ioctl(struct inode *inode, struct file *filp,
 		    unsigned int cmd, unsigned long arg,
 		    const struct ioctl_handler *handlers,
 		    long (*fallback)(const struct ioctl_arg *arg));
+long dispatch_ioctl_extensible(struct inode *inode, struct file *filp,
+			       unsigned int cmd, unsigned long arg,
+			       const struct ioctl_handler *handlers,
+			       long (*fallback)(const struct ioctl_arg *arg));
 
 #endif
 
-- 
1.6.0.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ