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, 10 Apr 2010 15:25:15 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Alexey Dobriyan <adobriyan@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Arnd Bergmann <arnd@...db.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	John Kacur <jkacur@...hat.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Al Viro <viro@...IV.linux.org.uk>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Ingo Molnar <mingo@...e.hu>
Subject: [PATCH v3] procfs: Push down the bkl from ioctl

Push down the bkl from procfs's ioctl main handler to its users.
Only three procfs users implement an ioctl (non unlocked) handler.
Turn them into unlocked_ioctl and push down the Devil inside.

v2: PDE(inode)->data doesn't need to be under bkl
v3: And don't forget to git-add the result

Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
Acked-by: Arnd Bergmann <arnd@...db.de>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Ingo Molnar <mingo@...e.hu>
Cc: John Kacur <jkacur@...hat.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc: Al Viro <viro@...IV.linux.org.uk>
Cc: Alexey Dobriyan <adobriyan@...il.com>
---
 drivers/char/i8k.c                  |   53 +++++++++++++++------
 drivers/isdn/divert/divert_procfs.c |   90 ++++++++++++++++++++++-------------
 net/sunrpc/cache.c                  |   17 +++++--
 3 files changed, 107 insertions(+), 53 deletions(-)

diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c
index fc8cf7a..85f8893 100644
--- a/drivers/char/i8k.c
+++ b/drivers/char/i8k.c
@@ -23,6 +23,7 @@
 #include <linux/seq_file.h>
 #include <linux/dmi.h>
 #include <linux/capability.h>
+#include <linux/smp_lock.h>
 #include <asm/uaccess.h>
 #include <asm/io.h>
 
@@ -82,8 +83,7 @@ module_param(fan_mult, int, 0);
 MODULE_PARM_DESC(fan_mult, "Factor to multiply fan speed with");
 
 static int i8k_open_fs(struct inode *inode, struct file *file);
-static int i8k_ioctl(struct inode *, struct file *, unsigned int,
-		     unsigned long);
+static long i8k_ioctl(struct file *, unsigned int, unsigned long);
 
 static const struct file_operations i8k_fops = {
 	.owner		= THIS_MODULE,
@@ -91,7 +91,7 @@ static const struct file_operations i8k_fops = {
 	.read		= seq_read,
 	.llseek		= seq_lseek,
 	.release	= single_release,
-	.ioctl		= i8k_ioctl,
+	.unlocked_ioctl	= i8k_ioctl,
 };
 
 struct smm_regs {
@@ -307,8 +307,7 @@ static int i8k_get_dell_signature(int req_fn)
 	return regs.eax == 1145651527 && regs.edx == 1145392204 ? 0 : -1;
 }
 
-static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd,
-		     unsigned long arg)
+static long i8k_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
 {
 	int val = 0;
 	int speed;
@@ -318,6 +317,9 @@ static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd,
 	if (!argp)
 		return -EINVAL;
 
+	/* Pushed down from procfs ioctl handler */
+	lock_kernel();
+
 	switch (cmd) {
 	case I8K_BIOS_VERSION:
 		val = i8k_get_bios_version();
@@ -341,57 +343,78 @@ static int i8k_ioctl(struct inode *ip, struct file *fp, unsigned int cmd,
 		break;
 
 	case I8K_GET_SPEED:
-		if (copy_from_user(&val, argp, sizeof(int)))
+		if (copy_from_user(&val, argp, sizeof(int))) {
+			unlock_kernel();
 			return -EFAULT;
+		}
 
 		val = i8k_get_fan_speed(val);
 		break;
 
 	case I8K_GET_FAN:
-		if (copy_from_user(&val, argp, sizeof(int)))
+		if (copy_from_user(&val, argp, sizeof(int))) {
+			unlock_kernel();
 			return -EFAULT;
+		}
 
 		val = i8k_get_fan_status(val);
 		break;
 
 	case I8K_SET_FAN:
-		if (restricted && !capable(CAP_SYS_ADMIN))
+		if (restricted && !capable(CAP_SYS_ADMIN)) {
+			unlock_kernel();
 			return -EPERM;
+		}
 
-		if (copy_from_user(&val, argp, sizeof(int)))
+		if (copy_from_user(&val, argp, sizeof(int))) {
+			unlock_kernel();
 			return -EFAULT;
+		}
 
-		if (copy_from_user(&speed, argp + 1, sizeof(int)))
+		if (copy_from_user(&speed, argp + 1, sizeof(int))) {
+			unlock_kernel();
 			return -EFAULT;
+		}
 
 		val = i8k_set_fan(val, speed);
 		break;
 
 	default:
+		unlock_kernel();
 		return -EINVAL;
 	}
 
-	if (val < 0)
-		return val;
+	if (val < 0) {
+		unlock_kernel();
+		return -val;
+	}
 
 	switch (cmd) {
 	case I8K_BIOS_VERSION:
-		if (copy_to_user(argp, &val, 4))
+		if (copy_to_user(argp, &val, 4)) {
+			unlock_kernel();
 			return -EFAULT;
+		}
 
 		break;
 	case I8K_MACHINE_ID:
-		if (copy_to_user(argp, buff, 16))
+		if (copy_to_user(argp, buff, 16)) {
+			unlock_kernel();
 			return -EFAULT;
+		}
 
 		break;
 	default:
-		if (copy_to_user(argp, &val, sizeof(int)))
+		if (copy_to_user(argp, &val, sizeof(int))) {
+			unlock_kernel();
 			return -EFAULT;
+		}
 
 		break;
 	}
 
+	unlock_kernel();
+
 	return 0;
 }
 
diff --git a/drivers/isdn/divert/divert_procfs.c b/drivers/isdn/divert/divert_procfs.c
index 3697c40..24b3642 100644
--- a/drivers/isdn/divert/divert_procfs.c
+++ b/drivers/isdn/divert/divert_procfs.c
@@ -19,6 +19,7 @@
 #include <linux/sched.h>
 #include <linux/isdnif.h>
 #include <net/net_namespace.h>
+#include <linux/smp_lock.h>
 #include "isdn_divert.h"
 
 
@@ -176,18 +177,20 @@ isdn_divert_close(struct inode *ino, struct file *filep)
 /*********/
 /* IOCTL */
 /*********/
-static int
-isdn_divert_ioctl(struct inode *inode, struct file *file,
-		  uint cmd, ulong arg)
+static long isdn_divert_ioctl(struct file *file, uint cmd, ulong arg)
 {
 	divert_ioctl dioctl;
-	int i;
 	unsigned long flags;
 	divert_rule *rulep;
 	char *cp;
 
-	if (copy_from_user(&dioctl, (void __user *) arg, sizeof(dioctl)))
+	/* Pushed down from procfs ioctl handler */
+	lock_kernel();
+
+	if (copy_from_user(&dioctl, (void __user *) arg, sizeof(dioctl))) {
+		unlock_kernel();
 		return -EFAULT;
+	}
 
 	switch (cmd) {
 		case IIOCGETVER:
@@ -195,65 +198,84 @@ isdn_divert_ioctl(struct inode *inode, struct file *file,
 			break;
 
 		case IIOCGETDRV:
-			if ((dioctl.getid.drvid = divert_if.name_to_drv(dioctl.getid.drvnam)) < 0)
-				return (-EINVAL);
+			if ((dioctl.getid.drvid = divert_if.name_to_drv(dioctl.getid.drvnam)) < 0) {
+				unlock_kernel();
+				return -EINVAL;
+			}
 			break;
 
 		case IIOCGETNAM:
 			cp = divert_if.drv_to_name(dioctl.getid.drvid);
-			if (!cp)
-				return (-EINVAL);
-			if (!*cp)
-				return (-EINVAL);
+			if (!cp || !*cp) {
+				unlock_kernel();
+				return -EINVAL;
+			}
 			strcpy(dioctl.getid.drvnam, cp);
 			break;
 
 		case IIOCGETRULE:
-			if (!(rulep = getruleptr(dioctl.getsetrule.ruleidx)))
-				return (-EINVAL);
+			if (!(rulep = getruleptr(dioctl.getsetrule.ruleidx))) {
+				unlock_kernel();
+				return -EINVAL;
+			}
 			dioctl.getsetrule.rule = *rulep;	/* copy data */
 			break;
 
 		case IIOCMODRULE:
-			if (!(rulep = getruleptr(dioctl.getsetrule.ruleidx)))
-				return (-EINVAL);
-            spin_lock_irqsave(&divert_lock, flags);
+			if (!(rulep = getruleptr(dioctl.getsetrule.ruleidx))) {
+				unlock_kernel();
+				return -EINVAL;
+			}
+			spin_lock_irqsave(&divert_lock, flags);
 			*rulep = dioctl.getsetrule.rule;	/* copy data */
 			spin_unlock_irqrestore(&divert_lock, flags);
-			return (0);	/* no copy required */
-			break;
+
+			unlock_kernel();
+			return 0;	/* no copy required */
 
 		case IIOCINSRULE:
-			return (insertrule(dioctl.getsetrule.ruleidx, &dioctl.getsetrule.rule));
-			break;
+			unlock_kernel();
+			return insertrule(dioctl.getsetrule.ruleidx, &dioctl.getsetrule.rule);
 
 		case IIOCDELRULE:
-			return (deleterule(dioctl.getsetrule.ruleidx));
-			break;
+			unlock_kernel();
+			return deleterule(dioctl.getsetrule.ruleidx);
 
 		case IIOCDODFACT:
-			return (deflect_extern_action(dioctl.fwd_ctrl.subcmd,
-						  dioctl.fwd_ctrl.callid,
-						 dioctl.fwd_ctrl.to_nr));
+			unlock_kernel();
+			return deflect_extern_action(dioctl.fwd_ctrl.subcmd,
+						     dioctl.fwd_ctrl.callid,
+						     dioctl.fwd_ctrl.to_nr);
 
 		case IIOCDOCFACT:
 		case IIOCDOCFDIS:
-		case IIOCDOCFINT:
-			if (!divert_if.drv_to_name(dioctl.cf_ctrl.drvid))
-				return (-EINVAL);	/* invalid driver */
-			if ((i = cf_command(dioctl.cf_ctrl.drvid,
+		case IIOCDOCFINT: {
+			long err;
+
+			if (!divert_if.drv_to_name(dioctl.cf_ctrl.drvid)) {
+				unlock_kernel();
+				return -EINVAL;	/* invalid driver */
+			}
+			err = cf_command(dioctl.cf_ctrl.drvid,
 					    (cmd == IIOCDOCFACT) ? 1 : (cmd == IIOCDOCFDIS) ? 0 : 2,
 					    dioctl.cf_ctrl.cfproc,
 					    dioctl.cf_ctrl.msn,
 					    dioctl.cf_ctrl.service,
 					    dioctl.cf_ctrl.fwd_nr,
-					    &dioctl.cf_ctrl.procid)))
-				return (i);
+					    &dioctl.cf_ctrl.procid);
+			if (err) {
+				unlock_kernel();
+				return err;
+			}
 			break;
 
+		}
 		default:
-			return (-EINVAL);
-	}			/* switch cmd */
+			unlock_kernel();
+			return -EINVAL;
+	}
+
+	unlock_kernel();
 	return copy_to_user((void __user *)arg, &dioctl, sizeof(dioctl)) ? -EFAULT : 0;
 }				/* isdn_divert_ioctl */
 
@@ -264,7 +286,7 @@ static const struct file_operations isdn_fops =
 	.read           = isdn_divert_read,
 	.write          = isdn_divert_write,
 	.poll           = isdn_divert_poll,
-	.ioctl          = isdn_divert_ioctl,
+	.unlocked_ioctl = isdn_divert_ioctl,
 	.open           = isdn_divert_open,
 	.release        = isdn_divert_close,                                      
 };
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 39bddba..3212357 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1331,12 +1331,21 @@ static unsigned int cache_poll_procfs(struct file *filp, poll_table *wait)
 	return cache_poll(filp, wait, cd);
 }
 
-static int cache_ioctl_procfs(struct inode *inode, struct file *filp,
-			      unsigned int cmd, unsigned long arg)
+static long cache_ioctl_procfs(struct file *filp,
+			       unsigned int cmd, unsigned long arg)
 {
+	long ret;
+	struct inode *inode = filp->f_path.dentry->d_inode;
 	struct cache_detail *cd = PDE(inode)->data;
 
-	return cache_ioctl(inode, filp, cmd, arg, cd);
+	/* Pushed down from procfs ioctl handler */
+	lock_kernel();
+
+	ret = cache_ioctl(inode, filp, cmd, arg, cd);
+
+	unlock_kernel();
+
+	return ret;
 }
 
 static int cache_open_procfs(struct inode *inode, struct file *filp)
@@ -1359,7 +1368,7 @@ static const struct file_operations cache_file_operations_procfs = {
 	.read		= cache_read_procfs,
 	.write		= cache_write_procfs,
 	.poll		= cache_poll_procfs,
-	.ioctl		= cache_ioctl_procfs, /* for FIONREAD */
+	.unlocked_ioctl	= cache_ioctl_procfs, /* for FIONREAD */
 	.open		= cache_open_procfs,
 	.release	= cache_release_procfs,
 };
-- 
1.6.2.3

--
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