Re: [PATCH] set*uid() must not fail-and-return on OOM/rlimits

From: Willy Tarreau
Date: Sun Aug 20 2006 - 04:24:53 EST

On Sun, Aug 20, 2006 at 04:38:40AM +0400, Solar Designer wrote:
> Willy and all,
> Attached is a trivial patch (extracted from 2.4.33-ow1) that makes
> set*uid() kill the current process rather than proceed with -EAGAIN when
> the kernel is running out of memory. Apparently, alloc_uid() can't fail
> and return anyway due to properties of the allocator, in which case the
> patch does not change a thing. But better safe than sorry.

Whether it can fail or not, alloc_uid()'s author intent was to report its
problems via NULL :

new = kmem_cache_alloc(uid_cachep, SLAB_KERNEL);
if (!new)
return NULL;

So your change to set_user() are consistent with this design choice.
Now, chosing to kill the process whe the kernel runs out of memory
seems consistent with what will happen a few milliseconds later to
other processes anyway.

I'm just wondering why you return a SIGSEGV. When the kernel kills
tasks on OOM conditions, it sends either SIGTERM or SIGKILL, as we
can see here in mm/oom_kill.c:__oom_kill_task() :

p->flags |= PF_MEMALLOC | PF_MEMDIE;
/* This process has hardware access, be more careful. */
if (cap_t(p->cap_effective) & CAP_TO_MASK(CAP_SYS_RAWIO)) {
force_sig(SIGTERM, p);
} else {
force_sig(SIGKILL, p);

Shouldn't we simply re-use the same code ? (not the function, I would not
like to get OOM messages outside the OOM killer).

> As you're probably aware, 2.6 kernels are affected to a greater extent,
> where set*uid() may also fail on trying to exceed RLIMIT_NPROC. That
> needs to be fixed, too.

I've followed the thread a little bit but am not aware of all the details.

> Opinions are welcome.
> Thanks,
> Alexander

What do you (and others) think about this ?

> diff -urpPX nopatch linux-2.4.33/kernel/sys.c linux/kernel/sys.c
> --- linux-2.4.33/kernel/sys.c Fri Nov 28 21:26:21 2003
> +++ linux/kernel/sys.c Wed Aug 16 05:19:21 2006
> @@ -514,8 +514,10 @@ static int set_user(uid_t new_ruid, int
> struct user_struct *new_user;
> new_user = alloc_uid(new_ruid);
> - if (!new_user)
> + if (!new_user) {
> + force_sig(SIGSEGV, current);
> return -EAGAIN;
> + }
> switch_uid(new_user);
> if(dumpclear)

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at