From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754177Ab2FSMXg (ORCPT ); Tue, 19 Jun 2012 08:23:36 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:52285 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751982Ab2FSMXe (ORCPT ); Tue, 19 Jun 2012 08:23:34 -0400 Date: Tue, 19 Jun 2012 07:23:20 -0500 From: Serge Hallyn To: Wanlong Gao Cc: mingo@kernel.org, hpa@zytor.com, linux-kernel@vger.kernel.org, dvhart@linux.intel.com, a.p.zijlstra@chello.nl, jkosina@suse.cz, ebiederm@xmission.com, dhowells@redhat.com, keescook@chromium.org, tglx@linutronix.de, linux-tip-commits@vger.kernel.org Subject: Re: [tip:core/locking] futex: Do not leak robust list to unprivileged process Message-ID: <20120619122320.GA3342@sergelap> References: <20120319231253.GA20893@www.outflux.net> <4FDFD8AF.6030209@cn.fujitsu.com> <20120619022456.GA2949@sergelap> <4FDFE4B1.80500@cn.fujitsu.com> <20120619031306.GA3985@sergelap> <4FDFF036.2000106@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FDFF036.2000106@cn.fujitsu.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > On 06/19/2012 11:13 AM, Serge Hallyn wrote: > > Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > >> On 06/19/2012 10:24 AM, Serge Hallyn wrote: > >>> Quoting Wanlong Gao (gaowanlong@cn.fujitsu.com): > >>>> On 03/29/2012 05:55 PM, tip-bot for Kees Cook wrote: > >>>>> Commit-ID: bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > >>>>> Gitweb: http://git.kernel.org/tip/bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8 > >>>>> Author: Kees Cook > >>>>> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700 > >>>>> Committer: Thomas Gleixner > >>>>> CommitDate: Thu, 29 Mar 2012 11:37:17 +0200 > >>>>> > >>>>> futex: Do not leak robust list to unprivileged process > >>>>> > >>>>> It was possible to extract the robust list head address from a setuid > >>>>> process if it had used set_robust_list(), allowing an ASLR info leak. This > >>>>> changes the permission checks to be the same as those used for similar > >>>>> info that comes out of /proc. > >>>>> > >>>>> Running a setuid program that uses robust futexes would have had: > >>>>> cred->euid != pcred->euid > >>>>> cred->euid == pcred->uid > >>>>> so the old permissions check would allow it. I'm not aware of any setuid > >>>>> programs that use robust futexes, so this is just a preventative measure. > >>>>> > >>>> > >>>> I'm not sure this change prevents the unprivileged process. > >>>> Please refer to LTP test, recently I saw that this change broke > >>>> the following test. > >>>> > >>>> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/get_robust_list/get_robust_list01.c#L155 > >>>> if (seteuid(1) == -1) > >>>> tst_brkm(TBROK|TERRNO, cleanup, "seteuid(1) failed"); > >>>> > >>>> TEST(retval = syscall(__NR_get_robust_list, 1, > >>>> (struct robust_list_head *)&head, > >>>> &len_ptr)); > >>>> > >>>> We set the euid to an unprivileged user, and expect to FAIL with EPERM, > >>>> without this patch, it FAIL as we expected, but with it, this call succeed. > >>> > >>> This relates to a question I asked - I believe in this thread, maybe in > >>> another thread - about ptrace_may_access. That code goes back further than > >>> our git history, and for so long has used current->uid and ->gid, not > >>> euid and gid, for permission checks. I asked if that's what we really > >>> want, but at the same am not sure we want to change something that's > >>> been like that for so long. > >>> > >>> But that's why it succeeded - you changed your euid, not your uid. > >> > >> Yeah, I known what I'm doing. > > > > Didn't mean to offend :) > > Sorry for my poor words, I didn't mean that, either. ;) > > > > >> I just wonder which is the right thing. > >> Should we check euid or uid ? You mean that checking uid instead of > >> checking euid for a long time, right? > > > > Yup, and I agree it seems wrong. > > Are there any other places where also switch checking uid instead of euid ? > In this place, anyway, this syscall is already marked as deprecated. This isn't just this syscall, though, it's ptrace_may_access() which is used in quite a few places (20 at quick glance). -serge