[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120619031306.GA3985@sergelap>
Date: Mon, 18 Jun 2012 22:13:06 -0500
From: Serge Hallyn <serge.hallyn@...onical.com>
To: Wanlong Gao <gaowanlong@...fujitsu.com>
Cc: mingo@...nel.org, hpa@...or.com, linux-kernel@...r.kernel.org,
dvhart@...ux.intel.com, a.p.zijlstra@...llo.nl, jkosina@...e.cz,
ebiederm@...ssion.com, dhowells@...hat.com, keescook@...omium.org,
tglx@...utronix.de, linux-tip-commits@...r.kernel.org
Subject: Re: [tip:core/locking] futex: Do not leak robust list to
unprivileged process
Quoting Wanlong Gao (gaowanlong@...fujitsu.com):
> On 06/19/2012 10:24 AM, Serge Hallyn wrote:
> > Quoting Wanlong Gao (gaowanlong@...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@...omium.org>
> >>> AuthorDate: Mon, 19 Mar 2012 16:12:53 -0700
> >>> Committer: Thomas Gleixner <tglx@...utronix.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 :)
> 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.
-serge
--
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/
Powered by blists - more mailing lists