On Mon, Jan 30, 2017 at 06:03:18PM +0100, Borislav Petkov wrote: > IOW, I'd like the user to know what she does and mean it. No sloppy > inputs.
Ok, here's what I wanna do. I decided to do my own parsing on the write path since proc_dostring() does not really allow me to look at the input string. Here's what I have so far, I'll do some more testing tomorrow. I know, the diff is practically unreadable so let me paste the whole function here. So this way I can really control which input is accepted and which not without jumping through hoops and doing silly games with the length. And of course I'm not saving/restoring strings like a crazy person. :-) Anyway, more fun tomorrow. int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write, void __user *buffer, size_t *lenp, loff_t *ppos) { char tmp_str[DEVKMSG_STR_MAX_SIZE]; unsigned int setting; int len, err; if (!write) { err = proc_dostring(table, write, buffer, lenp, ppos); if (err) return err; return 0; } if (devkmsg_log & DEVKMSG_LOG_MASK_LOCK) return -EINVAL; len = min_t(int, (int)*lenp, DEVKMSG_STR_MAX_SIZE); memset(tmp_str, 0, DEVKMSG_STR_MAX_SIZE); err = copy_from_user(tmp_str, buffer, len); if (err) return -EFAULT; err = __control_devkmsg(tmp_str, &setting); if (err < 0) return -EINVAL; /* known string */ if (err == len) goto assign; /* known string with a trailing \n */ if ((err + 1 == len) && (tmp_str[len - 1] == '\n')) goto assign; return -EINVAL; assign: if (devkmsg_log != setting) { memset(devkmsg_log_str, 0, DEVKMSG_STR_MAX_SIZE); strncpy(devkmsg_log_str, tmp_str, err); devkmsg_log = setting; } return 0; } --- diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 8b2696420abb..9bd84ca700d5 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -102,19 +102,19 @@ enum devkmsg_log_masks { static unsigned int __read_mostly devkmsg_log = DEVKMSG_LOG_MASK_DEFAULT; -static int __control_devkmsg(char *str) +static int __control_devkmsg(char *str, unsigned int *ret) { if (!str) return -EINVAL; if (!strncmp(str, "on", 2)) { - devkmsg_log = DEVKMSG_LOG_MASK_ON; + *ret = DEVKMSG_LOG_MASK_ON; return 2; } else if (!strncmp(str, "off", 3)) { - devkmsg_log = DEVKMSG_LOG_MASK_OFF; + *ret = DEVKMSG_LOG_MASK_OFF; return 3; } else if (!strncmp(str, "ratelimit", 9)) { - devkmsg_log = DEVKMSG_LOG_MASK_DEFAULT; + *ret = DEVKMSG_LOG_MASK_DEFAULT; return 9; } return -EINVAL; @@ -122,21 +122,25 @@ static int __control_devkmsg(char *str) static int __init control_devkmsg(char *str) { - if (__control_devkmsg(str) < 0) + unsigned int setting; + + if (__control_devkmsg(str, &setting) < 0) return 1; /* * Set sysctl string accordingly: */ - if (devkmsg_log == DEVKMSG_LOG_MASK_ON) { + if (setting == DEVKMSG_LOG_MASK_ON) { memset(devkmsg_log_str, 0, DEVKMSG_STR_MAX_SIZE); strncpy(devkmsg_log_str, "on", 2); - } else if (devkmsg_log == DEVKMSG_LOG_MASK_OFF) { + } else if (setting == DEVKMSG_LOG_MASK_OFF) { memset(devkmsg_log_str, 0, DEVKMSG_STR_MAX_SIZE); strncpy(devkmsg_log_str, "off", 3); } /* else "ratelimit" which is set by default. */ + devkmsg_log = setting; + /* * Sysctl cannot change it anymore. The kernel command line setting of * this parameter is to force the setting to be permanent throughout the @@ -154,37 +158,48 @@ char devkmsg_log_str[DEVKMSG_STR_MAX_SIZE] = "ratelimit"; 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; + char tmp_str[DEVKMSG_STR_MAX_SIZE]; + unsigned int setting; + int len, err; - if (write) { - if (devkmsg_log & DEVKMSG_LOG_MASK_LOCK) - return -EINVAL; + if (!write) { + err = proc_dostring(table, write, buffer, lenp, ppos); + if (err) + return err; - old = devkmsg_log; - strncpy(old_str, devkmsg_log_str, DEVKMSG_STR_MAX_SIZE); + return 0; } - err = proc_dostring(table, write, buffer, lenp, ppos); + if (devkmsg_log & DEVKMSG_LOG_MASK_LOCK) + return -EINVAL; + + len = min_t(int, (int)*lenp, DEVKMSG_STR_MAX_SIZE); + + memset(tmp_str, 0, DEVKMSG_STR_MAX_SIZE); + + err = copy_from_user(tmp_str, buffer, len); if (err) - return err; + return -EFAULT; - if (write) { - err = __control_devkmsg(devkmsg_log_str); + err = __control_devkmsg(tmp_str, &setting); + if (err < 0) + return -EINVAL; - /* - * Do not accept an unknown string OR a known string with - * trailing crap... - */ - if (err < 0 || (err + 1 != *lenp)) { + /* known string */ + if (err == len) + goto assign; - /* ... and restore old setting. */ - devkmsg_log = old; - strncpy(devkmsg_log_str, old_str, DEVKMSG_STR_MAX_SIZE); + /* known string with a trailing \n */ + if ((err + 1 == len) && (tmp_str[len - 1] == '\n')) + goto assign; - return -EINVAL; - } + return -EINVAL; + +assign: + if (devkmsg_log != setting) { + memset(devkmsg_log_str, 0, DEVKMSG_STR_MAX_SIZE); + strncpy(devkmsg_log_str, tmp_str, err); + devkmsg_log = setting; } return 0; -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --