kernel-hardening - Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110725234013.GB24110@openwall.com>
Date: Tue, 26 Jul 2011 03:40:13 +0400
From: Solar Designer <solar@...nwall.com>
To: Vasiliy Kulikov <segoon@...nwall.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
	NeilBrown <neilb@...e.de>, Stephen Smalley <sds@...ho.nsa.gov>,
	kernel-hardening@...ts.openwall.com,
	James Morris <jmorris@...ei.org>, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	Jiri Slaby <jslaby@...e.cz>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Eric Paris <eparis@...hat.com>, Willy Tarreau <w@....eu>,
	Sebastian Krahmer <krahmer@...e.de>
Subject: Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common()

Vasiliy,

On Mon, Jul 25, 2011 at 09:14:23PM +0400, Vasiliy Kulikov wrote:
> @@ -1433,6 +1433,19 @@ static int do_execve_common(const char *filename,
>  	struct files_struct *displaced;
>  	bool clear_in_exec;
>  	int retval;
> +	const struct cred *cred = current_cred();
> +
> +	/*
> +	 * We move the actual failure in case of RLIMIT_NPROC excess from
> +	 * set*uid() to execve() because too many poorly written programs
> +	 * don't check setuid() return code.  Here we additionally recheck
> +	 * whether NPROC limit is still exceeded.
> +	 */
> +	if ((current->flags & PF_NPROC_EXCEEDED) &&
> +	    atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) {
> +		retval = -EAGAIN;
> +		goto out_ret;
> +	}

Do you possibly need:

	current->flags &= ~PF_NPROC_EXCEEDED;

somewhere after this point?

I think it's weird to have past set_user() failure affect other than the
very next execve().

Perhaps also reset the flag on fork() because we have an RLIMIT_NPROC
check on fork() anyway.

Thanks,

Alexander

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.