Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754736Ab3JBQop (ORCPT ); Wed, 2 Oct 2013 12:44:45 -0400 Received: from mail-la0-f48.google.com ([209.85.215.48]:41528 "EHLO mail-la0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754109Ab3JBQok (ORCPT ); Wed, 2 Oct 2013 12:44:40 -0400 MIME-Version: 1.0 In-Reply-To: <20131002145506.GA2669@dztty> References: <1380659178-28605-1-git-send-email-tixxdz@opendz.org> <1380659178-28605-3-git-send-email-tixxdz@opendz.org> <524B78A2.40007@amacapital.net> <20131002145506.GA2669@dztty> From: Andy Lutomirski Date: Wed, 2 Oct 2013 17:44:17 +0100 Message-ID: Subject: Re: [PATCH v2 2/9] procfs: add proc_allow_access() to check if file's opener may access task To: Djalal Harouni Cc: "Eric W. Biederman" , Kees Cook , Al Viro , Andrew Morton , Linus Torvalds , Ingo Molnar , "Serge E. Hallyn" , Cyrill Gorcunov , David Rientjes , LKML , Linux FS Devel , kernel-hardening@lists.openwall.com, tixxdz@gmail.com Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3129 Lines: 70 On Wed, Oct 2, 2013 at 3:55 PM, Djalal Harouni wrote: > On Tue, Oct 01, 2013 at 06:36:34PM -0700, Andy Lutomirski wrote: >> On 10/01/2013 01:26 PM, Djalal Harouni wrote: >> > Since /proc entries varies at runtime, permission checks need to happen >> > during each system call. >> > >> > However even with that /proc file descriptors can be passed to a more >> > privileged process (e.g. a suid-exec) which will pass the classic >> > ptrace_may_access() permission check. The open() call will be issued in >> > general by an unprivileged process while the disclosure of sensitive >> > /proc information will happen using a more privileged process at >> > read(),write()... >> > >> > Therfore we need a more sophisticated check to detect if the cred of the >> > process have changed, and if the cred of the original opener that are >> > stored in the file->f_cred have enough permission to access the task's >> > /proc entries during read(), write()... >> > >> > Add the proc_allow_access() function that will receive the file->f_cred >> > as an argument, and tries to check if the opener had enough permission >> > to access the task's /proc entries. >> > >> > This function should be used with the ptrace_may_access() check. >> > >> > Cc: Kees Cook >> > Suggested-by: Eric W. Biederman >> > Signed-off-by: Djalal Harouni >> > --- >> > fs/proc/base.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> > fs/proc/internal.h | 2 ++ >> > 2 files changed, 58 insertions(+) >> > >> > diff --git a/fs/proc/base.c b/fs/proc/base.c >> > index e834946..c29eeae 100644 >> > --- a/fs/proc/base.c >> > +++ b/fs/proc/base.c >> > @@ -168,6 +168,62 @@ int proc_same_open_cred(const struct cred *fcred) >> > cap_issubset(cred->cap_permitted, fcred->cap_permitted)); >> > } >> > >> > +/* Returns 0 on success, -errno on denial. */ >> > +static int __proc_allow_access(const struct cred *cred, >> > + struct task_struct *task, unsigned int mode) >> > +{ >> > + int ret = 0; >> > + const struct cred *tcred; >> > + const struct cred *fcred = cred; >> > + >> > + rcu_read_lock(); >> > + tcred = __task_cred(task); >> > + if (uid_eq(fcred->uid, tcred->euid) && >> > + uid_eq(fcred->uid, tcred->suid) && >> > + uid_eq(fcred->uid, tcred->uid) && >> > + gid_eq(fcred->gid, tcred->egid) && >> > + gid_eq(fcred->gid, tcred->sgid) && >> > + gid_eq(fcred->gid, tcred->gid)) >> > + goto out; >> > + >> >> What's this for? Is it supposed to be an optimization? If so, it looks >> potentially exploitable, although I don't really understand what you're >> trying to do. > This function should be used in addition to the ptrace_may_access() one. Sorry, I was unclear. I meant: what are the uid and gid checks for? -- 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/