| 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
| ||
|
Date: Mon, 30 Jan 2017 23:39:12 +0100
From: Borislav Petkov <bp@...e.de>
To: Petr Mladek <pmladek@...e.com>
Cc: Rabin Vincent <rabin.vincent@...s.com>, akpm@...ux-foundation.org,
linux-kernel@...r.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [PATCH] printk: fix printk.devkmsg sysctl
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)
--
Powered by blists - more mailing lists