Messages in this thread | | | Date | Fri, 30 Jul 2021 01:23:29 -0700 | From | CGEL <> | Subject | Re: [PATCH] set_user: add capability check when rlimit(RLIMIT_NPROC) exceeds |
| |
On Wed, Jul 28, 2021 at 01:59:30PM +0200, Christian Brauner wrote: > [Ccing a few people that did the PF_NPROC_EXCEEDED changes] > > > Hey Cgel, > Hey Ran, > > The gist seems to me that this code wants to make sure that a program > can't successfully exec if it has gone through a set*id() transition > while exceeding its RLIMIT_NPROC. > > But I agree that the semantics here are a bit strange. > > Iicu, a capable but non-INIT_USER caller getting PF_NPROC_EXCEEDED set > during a set*id() transition wouldn't be able to exec right away if they > still exceed their RLIMIT_NPROC at the time of exec. So their exec would > fail in fs/exec.c: > > if ((current->flags & PF_NPROC_EXCEEDED) && > is_ucounts_overlimit(current_ucounts(), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) { > retval = -EAGAIN; > goto out_ret; > } > > However, if the caller were to fork() right after the set*id() > transition but before the exec while still exceeding their RLIMIT_NPROC > then they would get PF_NPROC_EXCEEDED cleared (while the child would > inherit it): > > retval = -EAGAIN; > if (is_ucounts_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) { > if (p->real_cred->user != INIT_USER && > !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) > goto bad_fork_free; > } > current->flags &= ~PF_NPROC_EXCEEDED; > > which means a subsequent exec by the capable caller would now succeed > even though they could still exceed their RLIMIT_NPROC limit. > > So at first glance, it seems that set_user() should probably get the > same check as it can be circumvented today unless I misunderstand the > original motivation. > > Christian
Hi Christian,
I think i didn't give enough information in the commit message. When switch to a capable but non-INIT_SUER and the RLIMIT_NPROC limit already exceeded, and calls these funcs: 1. set_xxuid()->exec() ---> fail 2. set_xxuid()->fork()->exec() ---> success Kernel should have the same behavior to uer space. Also i think non init_user CAN exceed the limit when with proper capability, so in the patch, set_user() clear PF_NPROC_EXCEEDED flag if capable() returns true.
| |