Commit 29541522 authored by Linus Torvalds's avatar Linus Torvalds

Merge branch 'proc-cmdline' (/proc/<pid>/cmdline fixes)

This fixes two problems reported with the cmdline simplification and
cleanup last year:

 - the setproctitle() special cases didn't quite match the original
   semantics, and it can be noticeable:

      https://lore.kernel.org/lkml/alpine.LNX.2.21.1904052326230.3249@kich.toxcorp.com/

 - it could leak an uninitialized byte from the temporary buffer under
   the right (wrong) circustances:

      https://lore.kernel.org/lkml/20190712160913.17727-1-izbyshev@ispras.ru/

It rewrites the logic entirely, splitting it into two separate commits
(and two separate functions) for the two different cases ("unedited
cmdline" vs "setproctitle() has been used to change the command line").

* proc-cmdline:
  /proc/<pid>/cmdline: add back the setproctitle() special case
  /proc/<pid>/cmdline: remove all the special cases
parents 50950626 d26d0cd9
...@@ -209,12 +209,53 @@ static int proc_root_link(struct dentry *dentry, struct path *path) ...@@ -209,12 +209,53 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
return result; return result;
} }
/*
* If the user used setproctitle(), we just get the string from
* user space at arg_start, and limit it to a maximum of one page.
*/
static ssize_t get_mm_proctitle(struct mm_struct *mm, char __user *buf,
size_t count, unsigned long pos,
unsigned long arg_start)
{
char *page;
int ret, got;
if (pos >= PAGE_SIZE)
return 0;
page = (char *)__get_free_page(GFP_KERNEL);
if (!page)
return -ENOMEM;
ret = 0;
got = access_remote_vm(mm, arg_start, page, PAGE_SIZE, FOLL_ANON);
if (got > 0) {
int len = strnlen(page, got);
/* Include the NUL character if it was found */
if (len < got)
len++;
if (len > pos) {
len -= pos;
if (len > count)
len = count;
len -= copy_to_user(buf, page+pos, len);
if (!len)
len = -EFAULT;
ret = len;
}
}
free_page((unsigned long)page);
return ret;
}
static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf, static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
size_t count, loff_t *ppos) size_t count, loff_t *ppos)
{ {
unsigned long arg_start, arg_end, env_start, env_end; unsigned long arg_start, arg_end, env_start, env_end;
unsigned long pos, len; unsigned long pos, len;
char *page; char *page, c;
/* Check if process spawned far enough to have cmdline. */ /* Check if process spawned far enough to have cmdline. */
if (!mm->env_end) if (!mm->env_end)
...@@ -231,28 +272,42 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf, ...@@ -231,28 +272,42 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
return 0; return 0;
/* /*
* We have traditionally allowed the user to re-write * We allow setproctitle() to overwrite the argument
* the argument strings and overflow the end result * strings, and overflow past the original end. But
* into the environment section. But only do that if * only when it overflows into the environment area.
* the environment area is contiguous to the arguments.
*/ */
if (env_start != arg_end || env_start >= env_end) if (env_start != arg_end || env_end < env_start)
env_start = env_end = arg_end; env_start = env_end = arg_end;
len = env_end - arg_start;
/* .. and limit it to a maximum of one page of slop */
if (env_end >= arg_end + PAGE_SIZE)
env_end = arg_end + PAGE_SIZE - 1;
/* We're not going to care if "*ppos" has high bits set */ /* We're not going to care if "*ppos" has high bits set */
pos = arg_start + *ppos; pos = *ppos;
if (pos >= len)
/* .. but we do check the result is in the proper range */
if (pos < arg_start || pos >= env_end)
return 0; return 0;
if (count > len - pos)
count = len - pos;
if (!count)
return 0;
/*
* Magical special case: if the argv[] end byte is not
* zero, the user has overwritten it with setproctitle(3).
*
* Possible future enhancement: do this only once when
* pos is 0, and set a flag in the 'struct file'.
*/
if (access_remote_vm(mm, arg_end-1, &c, 1, FOLL_ANON) == 1 && c)
return get_mm_proctitle(mm, buf, count, pos, arg_start);
/* .. and we never go past env_end */ /*
if (env_end - pos < count) * For the non-setproctitle() case we limit things strictly
count = env_end - pos; * to the [arg_start, arg_end[ range.
*/
pos += arg_start;
if (pos < arg_start || pos >= arg_end)
return 0;
if (count > arg_end - pos)
count = arg_end - pos;
page = (char *)__get_free_page(GFP_KERNEL); page = (char *)__get_free_page(GFP_KERNEL);
if (!page) if (!page)
...@@ -262,48 +317,11 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf, ...@@ -262,48 +317,11 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
while (count) { while (count) {
int got; int got;
size_t size = min_t(size_t, PAGE_SIZE, count); size_t size = min_t(size_t, PAGE_SIZE, count);
long offset;
/* got = access_remote_vm(mm, pos, page, size, FOLL_ANON);
* Are we already starting past the official end? if (got <= 0)
* We always include the last byte that is *supposed*
* to be NUL
*/
offset = (pos >= arg_end) ? pos - arg_end + 1 : 0;
got = access_remote_vm(mm, pos - offset, page, size + offset, FOLL_ANON);
if (got <= offset)
break; break;
got -= offset; got -= copy_to_user(buf, page, got);
/* Don't walk past a NUL character once you hit arg_end */
if (pos + got >= arg_end) {
int n = 0;
/*
* If we started before 'arg_end' but ended up
* at or after it, we start the NUL character
* check at arg_end-1 (where we expect the normal
* EOF to be).
*
* NOTE! This is smaller than 'got', because
* pos + got >= arg_end
*/
if (pos < arg_end)
n = arg_end - pos - 1;
/* Cut off at first NUL after 'n' */
got = n + strnlen(page+n, offset+got-n);
if (got < offset)
break;
got -= offset;
/* Include the NUL if it existed */
if (got < size)
got++;
}
got -= copy_to_user(buf, page+offset, got);
if (unlikely(!got)) { if (unlikely(!got)) {
if (!len) if (!len)
len = -EFAULT; len = -EFAULT;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment