From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:38570 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1752150AbcJCQDe (ORCPT <rfc822;linux-fsdevel@vger.kernel.org>);
        Mon, 3 Oct 2016 12:03:34 -0400
Date: Mon, 3 Oct 2016 18:02:20 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Jann Horn <jann@thejh.net>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Roland McGrath <roland@hack.frob.com>,
        John Johansen <john.johansen@canonical.com>,
        James Morris <james.l.morris@oracle.com>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        Paul Moore <aul@paul-moore.com>,
        Stephen Smalley <sds@tycho.nsa.gov>,
        Eric Paris <eparis@parisplace.org>,
        Casey Schaufler <casey@schaufler-ca.com>,
        Kees Cook <keescook@chromium.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Janis Danisevskis <jdanis@google.com>,
        Seth Forshee <seth.forshee@canonical.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Benjamin LaHaise <bcrl@kvack.org>,
        Ben Hutchings <ben@decadent.org.uk>,
        Andy Lutomirski <luto@amacapital.net>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        linux-fsdevel@vger.kernel.org,
        linux-security-module@vger.kernel.org, security@kernel.org
Subject: Re: [PATCH v2 1/8] exec: introduce cred_guard_light
Message-ID: <20161003160219.GB4758@redhat.com>
References: <1474663238-22134-1-git-send-email-jann@thejh.net> <1474663238-22134-2-git-send-email-jann@thejh.net> <20160930153505.GA17573@redhat.com> <87k2dtp7jj.fsf@x220.int.ebiederm.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87k2dtp7jj.fsf@x220.int.ebiederm.org>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On 09/30, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@redhat.com> writes:
>
> > And I think that cred_guard_mutex is already over-used in fs/proc. Say,
> > I think lock_trace() must die, I simply can't understand why it is useful.
> >
> > Suppose we modify, say, proc_pid_stack() to do
> >
> > 	save_stack_trace_tsk(task, &trace);
> > 	if (!ptrace_may_access(task, ...))
> > 		goto return -EPERM;
> >
> > 	for (i = 0; i < trace.nr_entries; i++)
> > 		seq_printf(...);
> >
> > 	return 0;
> >
> > is there any problem if it shows some trace before setuid exec does
> > install_exec_creds() ?
>
> You should make certain that the mm doesn't change in that picture,
> perhaps like /proc/<pid>/mem does.

Why? We do not care I think, /proc/pid/stack has nothing to do with
task->mm.

OK, we can use __ptrace_may_access() and call save_stack_trace_tsk()
under task_lock(), but I don't understand why should we worry.

And again, do you see any security problem with the code above? Yes,
it is "racy" but I think it is fine to occasionally succeed when it
races with credentials change. I fail to understand why the current
code abuses cred_guard_mutex to close the race with setuid exec.

Oleg.