Commit 72fa59970f8698023045ab0713d66f3f4f96945c

Authored by Vasiliy Kulikov
Committed by Linus Torvalds
1 parent 1d229d54db

move RLIMIT_NPROC check from set_user() to do_execve_common()

The patch http://lkml.org/lkml/2003/7/13/226 introduced an RLIMIT_NPROC
check in set_user() to check for NPROC exceeding via setuid() and
similar functions.

Before the check there was a possibility to greatly exceed the allowed
number of processes by an unprivileged user if the program relied on
rlimit only.  But the check created new security threat: many poorly
written programs simply don't check setuid() return code and believe it
cannot fail if executed with root privileges.  So, the check is removed
in this patch because of too often privilege escalations related to
buggy programs.

The NPROC can still be enforced in the common code flow of daemons
spawning user processes.  Most of daemons do fork()+setuid()+execve().
The check introduced in execve() (1) enforces the same limit as in
setuid() and (2) doesn't create similar security issues.

Neil Brown suggested to track what specific process has exceeded the
limit by setting PF_NPROC_EXCEEDED process flag.  With the change only
this process would fail on execve(), and other processes' execve()
behaviour is not changed.

Solar Designer suggested to re-check whether NPROC limit is still
exceeded at the moment of execve().  If the process was sleeping for
days between set*uid() and execve(), and the NPROC counter step down
under the limit, the defered execve() failure because NPROC limit was
exceeded days ago would be unexpected.  If the limit is not exceeded
anymore, we clear the flag on successful calls to execve() and fork().

The flag is also cleared on successful calls to set_user() as the limit
was exceeded for the previous user, not the current one.

Similar check was introduced in -ow patches (without the process flag).

v3 - clear PF_NPROC_EXCEEDED on successful calls to set_user().

Reviewed-by: James Morris <jmorris@namei.org>
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
Acked-by: NeilBrown <neilb@suse.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 5 changed files with 32 additions and 8 deletions Side-by-side Diff

... ... @@ -1459,6 +1459,23 @@
1459 1459 struct files_struct *displaced;
1460 1460 bool clear_in_exec;
1461 1461 int retval;
  1462 + const struct cred *cred = current_cred();
  1463 +
  1464 + /*
  1465 + * We move the actual failure in case of RLIMIT_NPROC excess from
  1466 + * set*uid() to execve() because too many poorly written programs
  1467 + * don't check setuid() return code. Here we additionally recheck
  1468 + * whether NPROC limit is still exceeded.
  1469 + */
  1470 + if ((current->flags & PF_NPROC_EXCEEDED) &&
  1471 + atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) {
  1472 + retval = -EAGAIN;
  1473 + goto out_ret;
  1474 + }
  1475 +
  1476 + /* We're below the limit (still or again), so we don't want to make
  1477 + * further execve() calls fail. */
  1478 + current->flags &= ~PF_NPROC_EXCEEDED;
1462 1479  
1463 1480 retval = unshare_files(&displaced);
1464 1481 if (retval)
include/linux/sched.h
... ... @@ -1767,6 +1767,7 @@
1767 1767 #define PF_DUMPCORE 0x00000200 /* dumped core */
1768 1768 #define PF_SIGNALED 0x00000400 /* killed by a signal */
1769 1769 #define PF_MEMALLOC 0x00000800 /* Allocating memory */
  1770 +#define PF_NPROC_EXCEEDED 0x00001000 /* set_user noticed that RLIMIT_NPROC was exceeded */
1770 1771 #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */
1771 1772 #define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */
1772 1773 #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */
... ... @@ -508,10 +508,8 @@
508 508 key_fsgid_changed(task);
509 509  
510 510 /* do it
511   - * - What if a process setreuid()'s and this brings the
512   - * new uid over his NPROC rlimit? We can check this now
513   - * cheaply with the new uid cache, so if it matters
514   - * we should be checking for it. -DaveM
  511 + * RLIMIT_NPROC limits on user->processes have already been checked
  512 + * in set_user().
515 513 */
516 514 alter_cred_subscribers(new, 2);
517 515 if (new->user != old->user)
... ... @@ -1111,6 +1111,7 @@
1111 1111 p->real_cred->user != INIT_USER)
1112 1112 goto bad_fork_free;
1113 1113 }
  1114 + current->flags &= ~PF_NPROC_EXCEEDED;
1114 1115  
1115 1116 retval = copy_creds(p, clone_flags);
1116 1117 if (retval < 0)
... ... @@ -621,11 +621,18 @@
621 621 if (!new_user)
622 622 return -EAGAIN;
623 623  
  624 + /*
  625 + * We don't fail in case of NPROC limit excess here because too many
  626 + * poorly written programs don't check set*uid() return code, assuming
  627 + * it never fails if called by root. We may still enforce NPROC limit
  628 + * for programs doing set*uid()+execve() by harmlessly deferring the
  629 + * failure to the execve() stage.
  630 + */
624 631 if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) &&
625   - new_user != INIT_USER) {
626   - free_uid(new_user);
627   - return -EAGAIN;
628   - }
  632 + new_user != INIT_USER)
  633 + current->flags |= PF_NPROC_EXCEEDED;
  634 + else
  635 + current->flags &= ~PF_NPROC_EXCEEDED;
629 636  
630 637 free_uid(new->user);
631 638 new->user = new_user;