[PATCH] printk: Fix printk.devkmsg sysctl

[Petr Mladek]

The code handling write into /proc/sys/kernel/printk_devkmsg expects
a new line at the end of the string but does not check it. As a result
it allows:

 # echo -n offX > /proc/sys/kernel/printk_devkmsg
 #

while at the same time it rejects legitimate uses:

 # echo -n off > /proc/sys/kernel/printk_devkmsg
 -sh: echo: write error: Invalid argument

This patch keeps using proc_dostring() to avoid a lot of duplicate code.
It increases the buffer size and allows to read one more character
beyond the string "ratelimit". In addition, it increases the limit
for strncmp() so that it checks also the trailing '\0' and do exact
matches.

Note that proc_dostring() checks for the trailing '\n' and does
not write it into the buffer.

As a result __control_devkmsg() is able to detect all misspelling.
and could return proper error codes. Also it never sets a wrong
devkmsg_log so that it need not be reverted later.

The code still does the ugly devkmsg_log_str write/restore.
But it looks like an acceptable deal.

Reported-by: Rabin Vincent <rabin.vincent@xxxxxxxx>
Signed-off-by: Petr Mladek <pmladek@xxxxxxxx>
---
 include/linux/printk.h |  4 ++--
 kernel/printk/printk.c | 33 +++++++++++----------------------
 2 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 3472cc6b7a60..c5aa0e268990 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -77,8 +77,8 @@ static inline void console_verbose(void)
 		console_loglevel = CONSOLE_LOGLEVEL_MOTORMOUTH;
 }
 
-/* strlen("ratelimit") + 1 */
-#define DEVKMSG_STR_MAX_SIZE 10
+/* strlen("ratelimit") + '\0' + one character to detect wrong entry */
+#define DEVKMSG_STR_MAX_SIZE 11
 extern char devkmsg_log_str[];
 struct ctl_table;
 
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8b2696420abb..7c56742a8baa 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -107,17 +107,17 @@ static int __control_devkmsg(char *str)
 	if (!str)
 		return -EINVAL;
 
-	if (!strncmp(str, "on", 2)) {
+	/* Do exact match by comparing also the trailing '\0'. */
+	if (!strncmp(str, "on", 3))
 		devkmsg_log = DEVKMSG_LOG_MASK_ON;
-		return 2;
-	} else if (!strncmp(str, "off", 3)) {
+	else if (!strncmp(str, "off", 4))
 		devkmsg_log = DEVKMSG_LOG_MASK_OFF;
-		return 3;
-	} else if (!strncmp(str, "ratelimit", 9)) {
+	else if (!strncmp(str, "ratelimit", 10))
 		devkmsg_log = DEVKMSG_LOG_MASK_DEFAULT;
-		return 9;
-	}
-	return -EINVAL;
+	else
+		return -EINVAL;
+
+	return 0;
 }
 
 static int __init control_devkmsg(char *str)
@@ -155,14 +155,12 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
 			      void __user *buffer, size_t *lenp, loff_t *ppos)
 {
 	char old_str[DEVKMSG_STR_MAX_SIZE];
-	unsigned int old;
 	int err;
 
 	if (write) {
 		if (devkmsg_log & DEVKMSG_LOG_MASK_LOCK)
 			return -EINVAL;
 
-		old = devkmsg_log;
 		strncpy(old_str, devkmsg_log_str, DEVKMSG_STR_MAX_SIZE);
 	}
 
@@ -173,21 +171,12 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
 	if (write) {
 		err = __control_devkmsg(devkmsg_log_str);
 
-		/*
-		 * Do not accept an unknown string OR a known string with
-		 * trailing crap...
-		 */
-		if (err < 0 || (err + 1 != *lenp)) {
-
-			/* ... and restore old setting. */
-			devkmsg_log = old;
+		/* Restore old setting when unknown parameter was used. */
+		if (err)
 			strncpy(devkmsg_log_str, old_str, DEVKMSG_STR_MAX_SIZE);
-
-			return -EINVAL;
-		}
 	}
 
-	return 0;
+	return err;
 }
 
 /*
-- 
1.8.5.6