Skip to content

Commit

Permalink
proc: environ_read() make sure offset points to environment address r…
Browse files Browse the repository at this point in the history
…ange

Currently the following offset and environment address range check in
environ_read() of /proc/<pid>/environ is buggy:

  int this_len = mm->env_end - (mm->env_start + src);
  if (this_len <= 0)
    break;

Large or negative offsets on /proc/<pid>/environ converted to 'unsigned
long' may pass this check since '(mm->env_start + src)' can overflow and
'this_len' will be positive.

This can turn /proc/<pid>/environ to act like /proc/<pid>/mem since
(mm->env_start + src) will point and read from another VMA.

There are two fixes here plus some code cleaning:

1) Fix the overflow by checking if the offset that was converted to
   unsigned long will always point to the [mm->env_start, mm->env_end]
   address range.

2) Remove the truncation that was made to the result of the check,
   storing the result in 'int this_len' will alter its value and we can
   not depend on it.

For kernels that have commit b409e57 ("proc: clean up
/proc/<pid>/environ handling") which adds the appropriate ptrace check and
saves the 'mm' at ->open() time, this is not a security issue.

This patch is taken from the grsecurity patch since it was just made
available.

Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Brad Spengler <spender@grsecurity.net>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Djalal Harouni authored and Linus Torvalds committed Jul 31, 2012
1 parent 108ceeb commit e8905ec
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions fs/proc/base.c
Original file line number Diff line number Diff line change
Expand Up @@ -827,15 +827,16 @@ static ssize_t environ_read(struct file *file, char __user *buf,
if (!atomic_inc_not_zero(&mm->mm_users))
goto free;
while (count > 0) {
int this_len, retval, max_len;
size_t this_len, max_len;
int retval;

this_len = mm->env_end - (mm->env_start + src);

if (this_len <= 0)
if (src >= (mm->env_end - mm->env_start))
break;

max_len = (count > PAGE_SIZE) ? PAGE_SIZE : count;
this_len = (this_len > max_len) ? max_len : this_len;
this_len = mm->env_end - (mm->env_start + src);

max_len = min_t(size_t, PAGE_SIZE, count);
this_len = min(max_len, this_len);

retval = access_remote_vm(mm, (mm->env_start + src),
page, this_len, 0);
Expand Down

0 comments on commit e8905ec

Please sign in to comment.