Subject: Re: [PATCH] set*uid() must not fail-and-return on OOM/rlimits
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Solar Designer <solar@openwall.com>
Cc: Willy Tarreau <wtarreau@hera.kernel.org>, linux-kernel@vger.kernel.org
In-Reply-To: <20060820221208.GA21754@openwall.com>
References: <20060820003840.GA17249@openwall.com>
	 <1156097640.4051.24.camel@localhost.localdomain>
	 <20060820221208.GA21754@openwall.com>
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Date: Sun, 20 Aug 2006 23:51:15 +0100
Message-Id: <1156114275.4051.71.camel@localhost.localdomain>
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2302
Lines: 52

Ar Llu, 2006-08-21 am 02:12 +0400, ysgrifennodd Solar Designer:
> Are you referring to killing of processes on OOM?  That was in Linux
> already, this patch does not introduce it.

(pedantic) Only if you have overcommit disabled.

> As it relates to setuid() in particular, POSIX.1-2001 says:
> 
>      The setuid() function shall fail, return -1, and set errno to the
>      corresponding value if one or more of the following are true:
> 
>      [EINVAL]
>              The value of the uid argument is invalid and not supported by
>              the implementation.
>      [EPERM]                                                                                 The process does not have appropriate privileges and uid does
>              not match the real user ID or the saved set-user-ID.
> 
> No other error conditions are defined.  

> I'd say that the behavior of returning EAGAIN is non-compliant.

You are allowed to return other errors. What you must not do is return a
different error for the description described in the text as I
understand it.

> But the kills are needed.  They are more correct and safer than
> returning EAGAIN.  An alternative would be to not allocate memory on
> set*uid() at all - like we did not in older kernels - but that would
> be an inappropriate behavior change for 2.4.

It is certainly an awkward case to get right when setuid code is not
being audited but I still think you are chasing the symptom, and its not
symptom of crap code, so you are not likely to "fix" security. A lot of
BSD code for example doesn't check malloc returns but you don't want an
auto-kill if mmap fails ?

The kill has the advantage that it stops the situation but it may also
be that you kill a program which can handle the case and you create a
new DoS attack (eg against a daemon switching to your uid). The current
situation is not good, the updated situation could be far worse.

The message is important, we want to know it happened in the memory
shortage case anyway.

Containers are also likely to create more such problems.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/