Date: Tue, 19 Jun 2012 07:23:20 -0500
From: Serge Hallyn <serge.hallyn@canonical.com>
To: Wanlong Gao <gaowanlong@cn.fujitsu.com>
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>
 <tip-bdbb776f882f5ad431aa1e694c69c1c3d6a4a5b8@git.kernel.org>
 <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
Content-Length: 3450
Lines: 76

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 <keescook@chromium.org>
> >>>>> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700
> >>>>> Committer:  Thomas Gleixner <tglx@linutronix.de>
> >>>>> 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
--
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/