RLIMIT_NPROC DoS fix

Hi,

There is a problem with RLIMIT_NPROC patch:
http://www.openwall.com/lists/kernel-hardening/2011/06/12/9
http://www.openwall.com/lists/kernel-hardening/2011/08/08/2

As planned, it moves RLIMIT check from set_user() (set*uid()) stage to fork()/execve() stage. But the drawback is that now there is no RLIMIT check at set_user(). It means that there are two bad cases:

1) a root process sometimes does setrlimit()+setuid(U)+execve()/fork(). In this case U can do kill(pid, SIGSTOP) each time just after setuid(U). U may store unlimited number of processes this way.
(Spender has learned this way.)

2) there are nonroot users A and B.
A makes a binary ./loop which does setuid(A)+for(;;) {} A does chmod u+s ./loop
B does 'while :; do ./loop &; done'

IOW, if there is a way one can produce unlimited number of processes at atfer-setuid before-execve stage, one may DoS the system.

I don't see any fair way to fix that. The way I see is using additional limit which is used to kill overmultiplied processes. E.g.:

set_user()

{
    ...
	if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) &&
			new_user != INIT_USER) {
		if (atomic_read(&new_user->processes) >= 2*rlimit(RLIMIT_NPROC) + 10) {
		    ...some ratelimited printk warning...
		    force_sig(SIGKILL, current);
		}
		current->flags |= PF_NPROC_EXCEEDED;
	} else {
		current->flags &= ~PF_NPROC_EXCEEDED;
    }
        
    ...
}

Yes, this is dirty. Any thoughts about less dirty way?

Vasily

I'd like to point out there are at least two other ways setuid() et alii can fail in recent kernels: LSM and memory allocation.

You could probably refuse to handle SIGSTOP sent by a nonprivileged user when PF_NPROC_EXCEEDED is set. I do not thing it would break anything. But it would make things even dirtier.

I think processes should count towards RLIMIT_NPROC of the user who forked them (B in this example) for the rest of their CPU cycles. Even after they exec a setuid program.

Create a cgroup controller limiting the number of tasks (and perhaps the number of other kernel objects) and pretend RLIMIT_NPROC has never existed?

Pavel Kankovsky aka Peak                          / Jeremiah 9:21        \
"For death is come up into our MS Windows(tm)..." \ 21st century edition /

Hi,

Let's keep this on kernel-hardening only (dropping owl-dev, adding Pavel).

FWIW, in pre-2.6 kernels there was no RLIMIT_NPROC enforcement on setuid() at all, so adding this check on execve() (like I did in -ow patches for 2.0-2.4 kernels) made things strictly more restricted. Now when we applied the same approach to mainline kernels, after RLIMIT_NPROC enforcement had been added to setuid(), we made things more relaxed in the way described by Vasily as compared to what they have been in 2.6 kernels for a long while. Yet I continue to think that the RLIMIT_NPROC enforcement on setuid() that existed in 2.6 kernels was wrong, and overall we're in a better state now.

LSM can break lots of things. ;-)

As to memory allocation, we previously determined (in last year's discussion on kernel-hardening) that it currently can't fail - yet I proposed (and I continue to propose) that we make the code fail-close in this respect (have the process killed if this currently impossible condition does occur, such as in a future code revision or if we made an error in our analysis). (We already have it that way - with process killing - in the patch applied on top of RHEL5/OpenVZ kernels in Owl.)

Yes, and we'd need to disallow setpriority() as well.

I think this is already the case, but the crucial detail about Vasily's example above is that the process itself fully switches to the target user with a setuid() call - and indeed we perform set_user() on setuid(). Should we start to treat setuid() from non-root real UID specially for the purpose of RLIMIT_NPROC? I think not.

Maybe we could postpone set_user() until execve() (which would never happen in the example above, thereby leaving the process B's for the purpose of RLIMIT_NPROC). This would deal with the SIGSTOP attack too.

This needs further consideration.

Alexander