Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757974AbZDUQqI (ORCPT ); Tue, 21 Apr 2009 12:46:08 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752909AbZDUQpu (ORCPT ); Tue, 21 Apr 2009 12:45:50 -0400 Received: from mx2.redhat.com ([66.187.237.31]:55407 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752913AbZDUQpt (ORCPT ); Tue, 21 Apr 2009 12:45:49 -0400 Date: Tue, 21 Apr 2009 18:39:57 +0200 From: Oleg Nesterov To: Hugh Dickins Cc: Al Viro , Linus Torvalds , Andrew Morton , Joe Malicki , Michael Itz , Kenneth Baker , Chris Wright , David Howells , Alexey Dobriyan , Greg Kroah-Hartman , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: Q: check_unsafe_exec() races (Was: [PATCH 2/4] fix setuid sometimes doesn't) Message-ID: <20090421163957.GD5402@redhat.com> References: <20090330011303.GN28946@ZenIV.linux.org.uk> <20090330013612.GA4080@redhat.com> <20090330014040.GA4807@redhat.com> <20090330123101.GQ28946@ZenIV.linux.org.uk> <20090331061615.GS28946@ZenIV.linux.org.uk> <20090401023849.GW28946@ZenIV.linux.org.uk> <20090406155103.GB21220@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2350 Lines: 58 On 04/19, Hugh Dickins wrote: > > 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. I like your patch more too. Except it has problems with unshare_fs() as Al pointed out. I agree, this fs->in_exec_wait_ptr looks really ugly, so... > 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. Perhaps I'll try to make the patch later, but I am not sure I send it ;) In any case it complicates the code. Oleg. -- 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/