linux-kernel - Re: Q: check_unsafe_exec() races (Was: [PATCH 2/4] fix setuid sometimes doesn't)

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: <Pine.LNX.4.64.0904191730130.9099@blonde.anvils>
Date:	Sun, 19 Apr 2009 17:44:28 +0100 (BST)
From:	Hugh Dickins <hugh@...itas.com>
To:	Oleg Nesterov <oleg@...hat.com>
cc:	Al Viro <viro@...IV.linux.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Joe Malicki <jmalicki@...acarta.com>,
	Michael Itz <mitz@...acarta.com>,
	Kenneth Baker <bakerk@...acarta.com>,
	Chris Wright <chrisw@...s-sol.org>,
	David Howells <dhowells@...hat.com>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: Q: check_unsafe_exec() races (Was: [PATCH 2/4] fix setuid
 sometimes doesn't)

On Mon, 6 Apr 2009, Oleg Nesterov wrote:
> On 04/01, Al Viro wrote:
> >
> > On Wed, Apr 01, 2009 at 01:28:01AM +0100, Hugh Dickins wrote:
> >
> > > Otherwise it looks good to me, except I keep worrying about those
> > > EAGAINs.
> >
> > Frankly, -EAGAIN in situation when we have userland race is fine.  And
> > we *do* have a userland race here - execve() will kill -9 those threads
> > in case of success, so if they'd been doing something useful, they are
> > about to be suddenly screwed.
> 
> Can't resist! I dislike the "in_exec && -EAGAIN" oddity too.
> 
> Yes sure, we can't break the "well written" applications. But imho this
> looks strange. And a bit "assymetrical" wrt LSM_UNSAFE_SHARE, I mean
> check_unsafe_exec() allows sub-threads to race or CLONE_FS but only if
> LSM_UNSAFE_SHARE.
> 
> Another reason, we can have the "my test-case found something strange"
> bug-reports.
> 
> So. Please feel free to nack or just ignore this message, but since I
> personally dislike the current behaviour I should at least try to suggest
> something else.

I didn't spend very long on this: it looked rather equivalent to the
current->fs->cred_exec_mutex patch that I proposed, but spinning its
own infrastructure rather than relying on the existing mutex (and of
course based on top of Al's patches, now in the tree, which have
changed the path of least resistance).

I've probably missed subtleties, but I still prefer my own suggestion;
though Al hinted at a subtle problem with that which I never grasped.

Of course I agree with you sharing my unease at -EAGAIN and in_exec.
Maybe we just lie in wait preparing a "told you so" for when someone
reports "something strange"!  But I'd really like to see your fix
patch go in.

Hugh
--
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/