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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20110715063113.GA3166@albatros>
Date:	Fri, 15 Jul 2011 10:31:13 +0400
From:	Vasiliy Kulikov <segoon@...nwall.com>
To:	kernel-hardening@...ts.openwall.com
Cc:	Solar Designer <solar@...nwall.com>,
	James Morris <jmorris@...ei.org>,
	Linus Torvalds <torvalds@...ux-foundation.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>,
	Stephen Smalley <sds@...ho.nsa.gov>, Willy Tarreau <w@....eu>,
	Sebastian Krahmer <krahmer@...e.de>
Subject: Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from
 set_user() to do_execve_common()

Neil,

On Fri, Jul 15, 2011 at 13:30 +1000, NeilBrown wrote:
> I'm still bothers that the proposed patch can cause exec to fail for an
> separate 'innocent' process.
> It also seems to 'hide' the problem - just shuffling code around.
> The comment in do_execve_common helps.  A similar comment in set_user
> wouldn't hurt.
> 
> But what do you think of this.  It sure that only the process which ignored
> the return value from setuid is inconvenienced.

I don't like it.  You're mixing the main problem and an RLIMIT check
enforcement.  The main goal is denying setuid() to fail unless there is not
enough privileges, RLIMIT in execve() is just an *attempt* to still count
NPROC in *some* widespread cases.  But you're trying to fix setuid()
where RLIMIT accounting is simple :\

Your patch doesn't address the core issue in this situation:

    setuid(); /* it fails because of RLIMIT */
    do_some_fs();
    execve();

do_some_fs() should be called ONLY if root is dropped.  In your scheme
the process may interact with FS as root while thinking it is nonroot,
which almost always leads to privilege escalation.

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/