linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Linux Containers <containers@lists.linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	Michal Hocko <mhocko@kernel.org>, Jann Horn <jann@thejh.net>,
	Willy Tarreau <w@1wt.eu>, Kees Cook <keescook@chromium.org>
Subject: Re: [REVIEW][PATCH 1/3] ptrace: Capture the ptracer's creds not PT_PTRACE_CAP
Date: Thu, 17 Nov 2016 15:27:09 -0800	[thread overview]
Message-ID: <CALCETrUSnPfzpabQMNuyOu09j9QDzRDeoQVF_U51=ow3bP5pkw@mail.gmail.com> (raw)
In-Reply-To: <87oa1eavfx.fsf_-_@xmission.com>

On Thu, Nov 17, 2016 at 9:05 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
>
> When the flag PT_PTRACE_CAP was added the PTRACE_TRACEME path was
> overlooked.  This can result in incorrect behavior when an application
> like strace traces an exec of a setuid executable.
>
> Further PT_PTRACE_CAP does not have enough information for making good
> security decisions as it does not report which user namespace the
> capability is in.  This has already allowed one mistake through
> insufficient granulariy.
>
> I found this issue when I was testing another corner case of exec and
> discovered that I could not get strace to set PT_PTRACE_CAP even when
> running strace as root with a full set of caps.
>
> This change fixes the above issue with strace allowing stracing as
> root a setuid executable without disabling setuid.  More fundamentaly
> this change allows what is allowable at all times, by using the correct
> information in it's decision.
>
> Cc: stable@vger.kernel.org
> Fixes: 4214e42f96d4 ("v2.4.9.11 -> v2.4.9.12")
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  fs/exec.c                  |  2 +-
>  include/linux/capability.h |  1 +
>  include/linux/ptrace.h     |  1 -
>  include/linux/sched.h      |  1 +
>  kernel/capability.c        | 20 ++++++++++++++++++++
>  kernel/ptrace.c            | 12 +++++++-----
>  6 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 6fcfb3f7b137..fdec760bfac3 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1401,7 +1401,7 @@ static void check_unsafe_exec(struct linux_binprm *bprm)
>         unsigned n_fs;
>
>         if (p->ptrace) {
> -               if (p->ptrace & PT_PTRACE_CAP)
> +               if (ptracer_capable(p, current_user_ns()))

IIRC PT_PTRACE_CAP was added to prevent TOCTOU races.  What prevents
that type of race now?  For that matter, what guarantees that we've
already switched to new creds here and will continue to do so in the
future?

--Andy

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2016-11-17 23:27 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-17 16:39 [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access Eric W. Biederman
2016-10-17 17:25 ` Jann Horn
2016-10-17 17:33   ` Eric W. Biederman
2016-10-18 13:50 ` Michal Hocko
2016-10-18 13:57   ` Jann Horn
2016-10-18 14:56   ` Eric W. Biederman
2016-10-18 15:05     ` Jann Horn
2016-10-18 15:35       ` Eric W. Biederman
2016-10-18 19:12         ` Jann Horn
2016-10-18 21:07           ` Eric W. Biederman
2016-10-18 21:15             ` [REVIEW][PATCH] exec: Don't exec files the userns root can not read Eric W. Biederman
2016-10-19  6:13               ` Amir Goldstein
2016-10-19 13:33                 ` Eric W. Biederman
2016-10-19 17:04                   ` Eric W. Biederman
2016-10-19 15:30               ` Andy Lutomirski
2016-10-19 16:52                 ` Eric W. Biederman
2016-10-19 17:29                   ` Jann Horn
2016-10-19 17:32                     ` Andy Lutomirski
2016-10-19 17:55                       ` Eric W. Biederman
2016-10-19 18:38                         ` Andy Lutomirski
2016-10-19 21:26                           ` Eric W. Biederman
2016-10-19 23:17                             ` Andy Lutomirski
2016-11-17 17:02                               ` [REVIEW][PATCH 0/3] Fixing ptrace vs exec vs userns interactions Eric W. Biederman
2016-11-17 17:05                                 ` [REVIEW][PATCH 1/3] ptrace: Capture the ptracer's creds not PT_PTRACE_CAP Eric W. Biederman
2016-11-17 23:14                                   ` Kees Cook
2016-11-18 18:56                                     ` Eric W. Biederman
2016-11-17 23:27                                   ` Andy Lutomirski [this message]
2016-11-17 23:44                                     ` Eric W. Biederman
2016-11-17 17:08                                 ` [REVIEW][PATCH 2/3] exec: Don't allow ptracing an exec of an unreadable file Eric W. Biederman
2016-11-17 20:47                                   ` Willy Tarreau
2016-11-17 21:07                                     ` Kees Cook
2016-11-17 21:32                                       ` Willy Tarreau
2016-11-17 21:51                                         ` Eric W. Biederman
2016-11-17 22:50                                           ` [REVIEW][PATCH 2/3] ptrace: Don't allow accessing an undumpable mm Eric W. Biederman
2016-11-17 23:17                                             ` Kees Cook
2016-11-17 23:28                                       ` [REVIEW][PATCH 2/3] exec: Don't allow ptracing an exec of an unreadable file Andy Lutomirski
2016-11-17 23:29                                   ` Andy Lutomirski
2016-11-17 23:55                                     ` Eric W. Biederman
2016-11-18  0:10                                       ` Andy Lutomirski
2016-11-18  0:35                                         ` Eric W. Biederman
2016-11-17 17:10                                 ` [REVIEW][PATCH 3/3] exec: Ensure mm->user_ns contains the execed files Eric W. Biederman
2016-11-19  7:17                                 ` [REVIEW][PATCH 0/3] Fixing ptrace vs exec vs userns interactions Willy Tarreau
2016-11-19  9:28                                   ` Willy Tarreau
2016-11-19  9:33                                     ` Willy Tarreau
2016-11-19 18:44                                     ` Eric W. Biederman
2016-11-19 18:35                                   ` Eric W. Biederman
2016-11-19 18:37                                     ` Eric W. Biederman
2016-10-19 18:36                   ` [REVIEW][PATCH] exec: Don't exec files the userns root can not read Andy Lutomirski
2016-10-18 18:06     ` [REVIEW][PATCH] mm: Add a user_ns owner to mm_struct and fix ptrace_may_access Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALCETrUSnPfzpabQMNuyOu09j9QDzRDeoQVF_U51=ow3bP5pkw@mail.gmail.com' \
    --to=luto@amacapital.net \
    --cc=containers@lists.linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=jann@thejh.net \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=oleg@redhat.com \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox