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-next>] [day] [month] [year] [list]
Date:	Tue, 06 Aug 2013 15:29:42 +0800
From:	Chen Gang <gang.chen@...anux.com>
To:	Al Viro <viro@...iv.linux.org.uk>,
	"Eric W. Biederman" <ebiederm@...ssion.com>, xi.wang@...il.com,
	nicolas.dichtel@...nd.com
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: [PATCH] kernel/sysctl_binary.c: improve the usage of return value
 'result'

Improve the usage of return value 'result', so not only can make code
clearer to readers, but also can improve the performance.

Assign the return error code only when error occurs, so can save some
instructions (some of them are inside looping).

Rewrite the sysctl_getname() to make code clearer to readers, and also
can save some instructions (it is mainly related with the usage of the
variable 'result').

Remove useless variable 'result' for sysctl() and compat sysctl(), when
do_sysctl() fails, it is meaningless to assign 'oldlenp ' with original
same value.

Also modify the related code to pass "./scripts/checkpatch.pl" checking.


Signed-off-by: Chen Gang <gang.chen@...anux.com>
---
 kernel/sysctl_binary.c |  142 ++++++++++++++++++++++++------------------------
 1 files changed, 72 insertions(+), 70 deletions(-)

diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index b609213..26d7fc0 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -941,15 +941,17 @@ static ssize_t bin_string(struct file *file,
 		copied = result;
 		lastp = oldval + copied - 1;
 
-		result = -EFAULT;
-		if (get_user(ch, lastp))
+		if (get_user(ch, lastp)) {
+			result = -EFAULT;
 			goto out;
+		}
 
 		/* Trim off the trailing newline */
 		if (ch == '\n') {
-			result = -EFAULT;
-			if (put_user('\0', lastp))
+			if (put_user('\0', lastp)) {
+				result = -EFAULT;
 				goto out;
+			}
 			copied -= 1;
 		}
 	}
@@ -974,10 +976,11 @@ static ssize_t bin_intvec(struct file *file,
 	char *buffer;
 	ssize_t result;
 
-	result = -ENOMEM;
 	buffer = kmalloc(BUFSZ, GFP_KERNEL);
-	if (!buffer)
+	if (!buffer) {
+		result = -ENOMEM;
 		goto out;
+	}
 
 	if (oldval && oldlen) {
 		unsigned __user *vec = oldval;
@@ -999,9 +1002,10 @@ static ssize_t bin_intvec(struct file *file,
 			while (isspace(*str))
 				str++;
 			
-			result = -EFAULT;
-			if (put_user(value, vec + i))
+			if (put_user(value, vec + i)) {
+				result = -EFAULT;
 				goto out_kfree;
+			}
 
 			copied += sizeof(*vec);
 			if (!isdigit(*str))
@@ -1020,10 +1024,10 @@ static ssize_t bin_intvec(struct file *file,
 		for (i = 0; i < length; i++) {
 			unsigned long value;
 
-			result = -EFAULT;
-			if (get_user(value, vec + i))
+			if (get_user(value, vec + i)) {
+				result = -EFAULT;
 				goto out_kfree;
-
+			}
 			str += snprintf(str, end - str, "%lu\t", value);
 		}
 
@@ -1045,10 +1049,11 @@ static ssize_t bin_ulongvec(struct file *file,
 	char *buffer;
 	ssize_t result;
 
-	result = -ENOMEM;
 	buffer = kmalloc(BUFSZ, GFP_KERNEL);
-	if (!buffer)
+	if (!buffer) {
+		result = -ENOMEM;
 		goto out;
+	}
 
 	if (oldval && oldlen) {
 		unsigned long __user *vec = oldval;
@@ -1070,9 +1075,10 @@ static ssize_t bin_ulongvec(struct file *file,
 			while (isspace(*str))
 				str++;
 			
-			result = -EFAULT;
-			if (put_user(value, vec + i))
+			if (put_user(value, vec + i)) {
+				result = -EFAULT;
 				goto out_kfree;
+			}
 
 			copied += sizeof(*vec);
 			if (!isdigit(*str))
@@ -1091,10 +1097,10 @@ static ssize_t bin_ulongvec(struct file *file,
 		for (i = 0; i < length; i++) {
 			unsigned long value;
 
-			result = -EFAULT;
-			if (get_user(value, vec + i))
+			if (get_user(value, vec + i)) {
+				result = -EFAULT;
 				goto out_kfree;
-
+			}
 			str += snprintf(str, end - str, "%lu\t", value);
 		}
 
@@ -1128,10 +1134,10 @@ static ssize_t bin_uuid(struct file *file,
 
 		/* Convert the uuid to from a string to binary */
 		for (i = 0; i < 16; i++) {
-			result = -EIO;
-			if (!isxdigit(str[0]) || !isxdigit(str[1]))
+			if (!isxdigit(str[0]) || !isxdigit(str[1])) {
+				result = -EIO;
 				goto out;
-
+			}
 			uuid[i] = (hex_to_bin(str[0]) << 4) |
 					hex_to_bin(str[1]);
 			str += 2;
@@ -1142,10 +1148,10 @@ static ssize_t bin_uuid(struct file *file,
 		if (oldlen > 16)
 			oldlen = 16;
 
-		result = -EFAULT;
-		if (copy_to_user(oldval, uuid, oldlen))
+		if (copy_to_user(oldval, uuid, oldlen)) {
+			result = -EFAULT;
 			goto out;
-
+		}
 		copied = oldlen;
 	}
 	result = copied;
@@ -1170,25 +1176,25 @@ static ssize_t bin_dn_node_address(struct file *file,
 		buf[result] = '\0';
 
 		/* Convert the decnet address to binary */
-		result = -EIO;
 		nodep = strchr(buf, '.');
-		if (!nodep)
+		if (!nodep) {
+			result = -EIO;
 			goto out;
+		}
 		++nodep;
 
 		area = simple_strtoul(buf, NULL, 10);
 		node = simple_strtoul(nodep, NULL, 10);
-
-		result = -EIO;
-		if ((area > 63)||(node > 1023))
+		if ((area > 63) || (node > 1023)) {
+			result = -EIO;
 			goto out;
-
+		}
 		dnaddr = cpu_to_le16((area << 10) | node);
 
-		result = -EFAULT;
-		if (put_user(dnaddr, (__le16 __user *)oldval))
+		if (put_user(dnaddr, (__le16 __user *)oldval)) {
+			result = -EFAULT;
 			goto out;
-
+		}
 		copied = sizeof(dnaddr);
 	}
 
@@ -1197,14 +1203,14 @@ static ssize_t bin_dn_node_address(struct file *file,
 		char buf[15];
 		int len;
 
-		result = -EINVAL;
-		if (newlen != sizeof(dnaddr))
+		if (newlen != sizeof(dnaddr)) {
+			result = -EINVAL;
 			goto out;
-
-		result = -EFAULT;
-		if (get_user(dnaddr, (__le16 __user *)newval))
+		}
+		if (get_user(dnaddr, (__le16 __user *)newval)) {
+			result = -EFAULT;
 			goto out;
-
+		}
 		len = snprintf(buf, sizeof(buf), "%hu.%hu",
 				le16_to_cpu(dnaddr) >> 10,
 				le16_to_cpu(dnaddr) & 0x3ff);
@@ -1276,19 +1282,20 @@ repeat:
 
 static char *sysctl_getname(const int *name, int nlen, const struct bin_table **tablep)
 {
-	char *tmp, *result;
-
-	result = ERR_PTR(-ENOMEM);
-	tmp = __getname();
-	if (tmp) {
-		const struct bin_table *table = get_sysctl(name, nlen, tmp);
-		result = tmp;
-		*tablep = table;
-		if (IS_ERR(table)) {
-			__putname(tmp);
-			result = ERR_CAST(table);
-		}
+	char *result;
+	const struct bin_table *table;
+
+	result = __getname();
+	if (!result)
+		return ERR_PTR(-ENOMEM);
+
+	table = get_sysctl(name, nlen, result);
+	if (IS_ERR(table)) {
+		__putname(result);
+		return ERR_CAST(table);
 	}
+	*tablep = table;
+
 	return result;
 }
 
@@ -1303,9 +1310,10 @@ static ssize_t binary_sysctl(const int *name, int nlen,
 	int flags;
 
 	pathname = sysctl_getname(name, nlen, &table);
-	result = PTR_ERR(pathname);
-	if (IS_ERR(pathname))
+	if (IS_ERR(pathname)) {
+		result = PTR_ERR(pathname);
 		goto out;
+	}
 
 	/* How should the sysctl be accessed? */
 	if (oldval && oldlen && newval && newlen) {
@@ -1321,10 +1329,10 @@ static ssize_t binary_sysctl(const int *name, int nlen,
 
 	mnt = task_active_pid_ns(current)->proc_mnt;
 	file = file_open_root(mnt->mnt_root, mnt, pathname, flags);
-	result = PTR_ERR(file);
-	if (IS_ERR(file))
+	if (IS_ERR(file)) {
+		result = PTR_ERR(file);
 		goto out_putname;
-
+	}
 	result = table->convert(file, oldval, oldlen, newval, newlen);
 
 	fput(file);
@@ -1420,7 +1428,6 @@ SYSCALL_DEFINE1(sysctl, struct __sysctl_args __user *, args)
 {
 	struct __sysctl_args tmp;
 	size_t oldlen = 0;
-	ssize_t result;
 
 	if (copy_from_user(&tmp, args, sizeof(tmp)))
 		return -EFAULT;
@@ -1431,18 +1438,16 @@ SYSCALL_DEFINE1(sysctl, struct __sysctl_args __user *, args)
 	if (tmp.oldlenp && get_user(oldlen, tmp.oldlenp))
 		return -EFAULT;
 
-	result = do_sysctl(tmp.name, tmp.nlen, tmp.oldval, oldlen,
+	oldlen = do_sysctl(tmp.name, tmp.nlen, tmp.oldval, oldlen,
 			   tmp.newval, tmp.newlen);
 
-	if (result >= 0) {
-		oldlen = result;
-		result = 0;
-	}
+	if ((ssize_t)oldlen < 0)
+		return (ssize_t)oldlen;
 
 	if (tmp.oldlenp && put_user(oldlen, tmp.oldlenp))
 		return -EFAULT;
 
-	return result;
+	return 0;
 }
 
 
@@ -1463,7 +1468,6 @@ COMPAT_SYSCALL_DEFINE1(sysctl, struct compat_sysctl_args __user *, args)
 	struct compat_sysctl_args tmp;
 	compat_size_t __user *compat_oldlenp;
 	size_t oldlen = 0;
-	ssize_t result;
 
 	if (copy_from_user(&tmp, args, sizeof(tmp)))
 		return -EFAULT;
@@ -1475,19 +1479,17 @@ COMPAT_SYSCALL_DEFINE1(sysctl, struct compat_sysctl_args __user *, args)
 	if (compat_oldlenp && get_user(oldlen, compat_oldlenp))
 		return -EFAULT;
 
-	result = do_sysctl(compat_ptr(tmp.name), tmp.nlen,
+	oldlen = do_sysctl(compat_ptr(tmp.name), tmp.nlen,
 			   compat_ptr(tmp.oldval), oldlen,
 			   compat_ptr(tmp.newval), tmp.newlen);
 
-	if (result >= 0) {
-		oldlen = result;
-		result = 0;
-	}
+	if ((ssize_t)oldlen < 0)
+		return (ssize_t)oldlen;
 
 	if (compat_oldlenp && put_user(oldlen, compat_oldlenp))
 		return -EFAULT;
 
-	return result;
+	return 0;
 }
 
 #endif /* CONFIG_COMPAT */
-- 
1.7.7.6

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