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.
Unified · Split
| @@ -1717,21 +1717,18 @@ static int _proc_do_string(char *data, int maxlen, int write, · 1716 unmodified lines | ||
| 1717 | 1717 | } |
| 1718 | 1718 | |
| 1719 | 1719 | if (write) { |
| 1720 | + /* Start writing from beginning of buffer. */ | |
| 1720 | 1721 | len = 0; |
| 1722 | + *ppos += *lenp; | |
| 1721 | 1723 | p = buffer; |
| 1722 | − while (len < *lenp) { | |
| 1724 | + while ((p - buffer) < *lenp && len < maxlen - 1) { | |
| 1723 | 1725 | if (get_user(c, p++)) |
| 1724 | 1726 | return -EFAULT; |
| 1725 | 1727 | if (c == 0 || c == '\n') |
| 1726 | 1728 | break; |
| 1727 | − len++; | |
| 1729 | + data[len++] = c; | |
| 1728 | 1730 | } |
| 1729 | − if (len >= maxlen) | |
| 1730 | − len = maxlen-1; | |
| 1731 | − if(copy_from_user(data, buffer, len)) | |
| 1732 | − return -EFAULT; | |
| 1733 | 1731 | data[len] = 0; |
| 1734 | − *ppos += *lenp; | |
| 1735 | 1732 | } else { |
| 1736 | 1733 | len = strlen(data); |
| 1737 | 1734 | if (len > maxlen) |