From 2ca9bb456ada8bcbdc8f77f8fc78207653bbaa92 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 6 Jun 2014 14:37:18 -0700 Subject: [PATCH] sysctl: refactor sysctl string writing logic Consolidate buffer length checking with new-line/end-of-line checking. Additionally, instead of reading user memory twice, just do the assignment during the loop. This change doesn't affect the potential races here. It was already possible to read a sysctl that was in the middle of a write. In both cases, the string will always be NULL terminated. The pre-existing race remains a problem to be solved. Signed-off-by: Kees Cook Cc: Randy Dunlap Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/sysctl.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 3e214beabbe9..ac6847feaa83 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1717,21 +1717,18 @@ static int _proc_do_string(char *data, int maxlen, int write, } if (write) { + /* Start writing from beginning of buffer. */ len = 0; + *ppos += *lenp; p = buffer; - while (len < *lenp) { + while ((p - buffer) < *lenp && len < maxlen - 1) { if (get_user(c, p++)) return -EFAULT; if (c == 0 || c == '\n') break; - len++; + data[len++] = c; } - if (len >= maxlen) - len = maxlen-1; - if(copy_from_user(data, buffer, len)) - return -EFAULT; data[len] = 0; - *ppos += *lenp; } else { len = strlen(data); if (len > maxlen)