Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754763Ab0GHJdb (ORCPT ); Thu, 8 Jul 2010 05:33:31 -0400 Received: from mx2.mail.elte.hu ([157.181.151.9]:58021 "EHLO mx2.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754512Ab0GHJda (ORCPT ); Thu, 8 Jul 2010 05:33:30 -0400 Date: Thu, 8 Jul 2010 11:32:41 +0200 From: Ingo Molnar To: Michal Hocko Cc: Linus Torvalds , Thomas Gleixner , LKML , Nick Piggin , Alexey Kuznetsov , Peter Zijlstra , Darren Hart Subject: Re: [PATCH] futex: futex_find_get_task remove credentails check Message-ID: <20100708093241.GB6057@elte.hu> References: <20100628153214.GA24127@tiehlicka.suse.cz> <20100628155845.GC24127@tiehlicka.suse.cz> <20100628163952.GD24127@tiehlicka.suse.cz> <1277743748.3561.139.camel@laptop> <20100629084230.GE6215@tiehlicka.suse.cz> <20100630070115.GB3890@tiehlicka.suse.cz> <20100630095525.GA5840@tiehlicka.suse.cz> <4C2B742F.6050403@us.ibm.com> <20100708092819.GB4925@tiehlicka.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100708092819.GB4925@tiehlicka.suse.cz> User-Agent: Mutt/1.5.20 (2009-08-17) X-ELTE-SpamScore: 1.0 X-ELTE-SpamLevel: s X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=1.0 required=5.9 tests=BAYES_50 autolearn=no SpamAssassin version=3.2.5 1.0 BAYES_50 BODY: Bayesian spam probability is 40 to 60% [score: 0.4709] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3385 Lines: 73 * Michal Hocko wrote: > On Wed 30-06-10 09:43:27, Darren Hart wrote: > > On 06/30/2010 02:55 AM, Michal Hocko wrote: > > >On Wed 30-06-10 09:01:15, Michal Hocko wrote: > > >>On Tue 29-06-10 09:41:02, Linus Torvalds wrote: > > >>>On Tue, Jun 29, 2010 at 1:42 AM, Michal Hocko wrote: > > >>>> > > >>>>futex_find_get_task is currently used (through lookup_pi_state) from two > > >>>>contexts, futex_requeue and futex_lock_pi_atomic. While credentials check > > >>>>makes sense in the first code path, the second one is more problematic > > >>>>because this check requires that the PI lock holder (pid parameter) has > > >>>>the same uid and euid as the process's euid which is trying to lock the > > >>>>same futex (current). > > >>> > > >>>So exactly why does it make sense to check the credentials in the > > >>>first code path then? > > >> > > >>I though that requeue needs this for security reasons (don't let requeue > > >>process for other user), but when I thought about that again you are > > >>right and the only what matters should be accessibility of the shared > > >>memory. > > > > > >And here is the patch which does the thing. > > > > > >-- > > > > > > From 082c5ad2c482a8e78b61b17e213e750b006176aa Mon Sep 17 00:00:00 2001 > > >From: Michal Hocko > > >Date: Wed, 30 Jun 2010 09:51:19 +0200 > > >Subject: [PATCH] futex: futex_find_get_task remove credentails check > > > > > >futex_find_get_task is currently used (through lookup_pi_state) from two > > >contexts, futex_requeue and futex_lock_pi_atomic. None of the paths > > >looks it needs the credentials check, though. Different (e)uids > > >shouldn't matter at all because the only thing that is important for > > >shared futex is the accessibility of the shared memory. > > > > > >The credentail check results in glibc assert failure or process hang (if > > >glibc is compiled without assert support) for shared robust pthread > > >mutex with priority inheritance if a process tries to lock already held > > >lock owned by a process with a different euid: > > > > > >pthread_mutex_lock.c:312: __pthread_mutex_lock_full: Assertion `(-(e)) != 3 || !robust' failed. > > > > > >The problem is that futex_lock_pi_atomic which is called when we try to > > >lock already held lock checks the current holder (tid is stored in the > > >futex value) to get the PI state. It uses lookup_pi_state which in turn > > >gets task struct from futex_find_get_task. ESRCH is returned either when > > >the task is not found or if credentials check fails. > > >futex_lock_pi_atomic simply returns if it gets ESRCH. glibc code, > > >however, doesn't expect that robust lock returns with ESRCH because it > > >should get either success or owner died. > > > > > >Signed-off-by: Michal Hocko > > > > Without hearing back from Ingo on the original intent of the > > credentials check, this looks right to me. > > Could you comment on that Ingo, please? I think that's more of a question to Thomas :-) My memories are hazy and nothing springs out as some credible original intent. So please assume it doesnt exist :-) Ingo -- 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/