LKML Archive on lore.kernel.org
 help / color / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: Christian Brauner <christian.brauner@ubuntu.com>,
	Kees Cook <keescook@chromium.org>, Jann Horn <jannh@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Oleg Nesterov <oleg@redhat.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Andrei Vagin <avagin@gmail.com>, Ingo Molnar <mingo@kernel.org>,
	"Peter Zijlstra \(Intel\)" <peterz@infradead.org>,
	Yuyang Du <duyuyang@gmail.com>,
	David Hildenbrand <david@redhat.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	David Howells <dhowells@redhat.com>,
	James Morris <jamorris@linux.microsoft.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Shakeel Butt <shakeelb@google.com>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Christian Kellner <christian@kellner.me>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Aleksa Sarai <cyphar@cyphar.com>,
	"Dmitry V. Levin" <ldv@altlinux.org>,
	"linux-doc\@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-fsdevel\@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-mm\@kvack.org" <linux-mm@kvack.org>,
	"stable\@vger.kernel.org" <stable@vger.kernel.org>,
	"linux-api\@vger.kernel.org" <linux-api@vger.kernel.org>
Subject: Re: [PATCH 3/4] proc: io_accounting: Use new infrastructure to fix deadlocks in execve
Date: Tue, 10 Mar 2020 14:06:36 -0500
Message-ID: <874kuwvxkz.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <AM6PR03MB5170BD2476E35068E182EFA4E4FF0@AM6PR03MB5170.eurprd03.prod.outlook.com> (Bernd Edlinger's message of "Tue, 10 Mar 2020 18:45:47 +0100")

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> This changes do_io_accounting to use the new exec_update_mutex
> instead of cred_guard_mutex.
>
> This fixes possible deadlocks when the trace is accessing
> /proc/$pid/io for instance.
>
> This should be safe, as the credentials are only used for reading.

This is an improvement.

We probably want to do this just as an incremental step in making things
better but perhaps I am blind but I am not finding the reason for
guarding this with the cred_guard_mutex to be at all persuasive.

I think moving the ptrace_may_access check down to after the
unlock_task_sighand would be just as effective at addressing the
concerns raised in the original commit.  I think the task_lock provides
all of the barrier we need to make it safe to move the ptrace_may_access
checks safe.

The reason I say this is I don't see exec changing ->ioac.  Just
performing some I/O which would update the io accounting statistics.

Can anyone see if I am wrong?

Eric


commit 293eb1e7772b25a93647c798c7b89bf26c2da2e0
Author: Vasiliy Kulikov <segoon@openwall.com>
Date:   Tue Jul 26 16:08:38 2011 -0700

    proc: fix a race in do_io_accounting()
    
    If an inode's mode permits opening /proc/PID/io and the resulting file
    descriptor is kept across execve() of a setuid or similar binary, the
    ptrace_may_access() check tries to prevent using this fd against the
    task with escalated privileges.
    
    Unfortunately, there is a race in the check against execve().  If
    execve() is processed after the ptrace check, but before the actual io
    information gathering, io statistics will be gathered from the
    privileged process.  At least in theory this might lead to gathering
    sensible information (like ssh/ftp password length) that wouldn't be
    available otherwise.
    
    Holding task->signal->cred_guard_mutex while gathering the io
    information should protect against the race.
    
    The order of locking is similar to the one inside of ptrace_attach():
    first goes cred_guard_mutex, then lock_task_sighand().
    
    Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Cc: <stable@kernel.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>



> Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
> ---
>  fs/proc/base.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 4fdfe4f..529d0c6 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2770,7 +2770,7 @@ static int do_io_accounting(struct task_struct *task, struct seq_file *m, int wh
>  	unsigned long flags;
>  	int result;
>  
> -	result = mutex_lock_killable(&task->signal->cred_guard_mutex);
> +	result = mutex_lock_killable(&task->signal->exec_update_mutex);
>  	if (result)
>  		return result;
>  
> @@ -2806,7 +2806,7 @@ static int do_io_accounting(struct task_struct *task, struct seq_file *m, int wh
>  	result = 0;
>  
>  out_unlock:
> -	mutex_unlock(&task->signal->cred_guard_mutex);
> +	mutex_unlock(&task->signal->exec_update_mutex);
>  	return result;
>  }

  reply index

Thread overview: 203+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-01 11:27 [PATCH] exec: Fix a deadlock in ptrace Bernd Edlinger
2020-03-01 15:13 ` Aleksa Sarai
2020-03-01 15:58   ` Christian Brauner
2020-03-01 17:46     ` Bernd Edlinger
2020-03-01 18:20       ` Christian Brauner
2020-03-01 17:24   ` Bernd Edlinger
2020-03-01 18:21 ` Jann Horn
2020-03-01 18:52   ` Christian Brauner
2020-03-01 19:00     ` Bernd Edlinger
2020-03-01 20:00     ` Jann Horn
2020-03-01 20:34       ` [PATCHv2] " Bernd Edlinger
2020-03-02  6:38         ` Eric W. Biederman
2020-03-02 15:43           ` Bernd Edlinger
2020-03-02 15:57             ` Eric W. Biederman
2020-03-02 16:02               ` Bernd Edlinger
2020-03-02 16:17                 ` Eric W. Biederman
2020-03-02 16:43                   ` Jann Horn
2020-03-02 17:01                     ` Bernd Edlinger
2020-03-02 17:37                       ` Jann Horn
2020-03-02 17:42                         ` christian
2020-03-02 18:08                           ` Jann Horn
2020-03-02 20:10                             ` [PATCHv3] " Bernd Edlinger
2020-03-02 20:28                               ` Bernd Edlinger
2020-03-02 17:13                   ` [PATCHv2] " Bernd Edlinger
2020-03-02 21:49                     ` Eric W. Biederman
2020-03-02 22:00                       ` Bernd Edlinger
2020-03-02 22:18                       ` [PATCHv4] " Bernd Edlinger
2020-03-03  2:26                         ` Kees Cook
2020-03-03  4:54                           ` Bernd Edlinger
2020-03-03  5:29                             ` Kees Cook
2020-03-03  8:08                               ` Bernd Edlinger
2020-03-03  8:34                                 ` Christian Brauner
2020-03-03  8:43                                   ` Christian Brauner
2020-03-04 15:30                                 ` Christian Brauner
2020-03-03  8:58                           ` Christian Brauner
2020-03-03 10:34                             ` Bernd Edlinger
2020-03-03 11:23                               ` Bernd Edlinger
2020-03-03 14:20                                 ` Christian Brauner
2020-03-03 13:02                             ` [PATCHv5] " Bernd Edlinger
2020-03-03 15:18                               ` Eric W. Biederman
2020-03-03 16:48                                 ` Bernd Edlinger
2020-03-03 17:01                                   ` Christian Brauner
2020-03-03 17:20                                     ` Christian Brauner
2020-03-03 20:08                                   ` Eric W. Biederman
2020-03-04 14:37                                     ` Bernd Edlinger
2020-03-04 16:33                                       ` Eric W. Biederman
2020-03-04 21:49                                         ` Bernd Edlinger
2020-03-04 21:56                                         ` [PATCHv6] " Bernd Edlinger
2020-03-05 18:36                                           ` Bernd Edlinger
2020-03-05 21:14                                             ` [PATCH 0/2] Infrastructure to allow fixing exec deadlocks Eric W. Biederman
2020-03-05 21:15                                               ` [PATCH 1/2] exec: Properly mark the point of no return Eric W. Biederman
2020-03-05 22:34                                                 ` Bernd Edlinger
2020-03-06  5:19                                                   ` Eric W. Biederman
2020-03-05 22:56                                                 ` Bernd Edlinger
2020-03-06  5:09                                                   ` Eric W. Biederman
2020-03-06 16:26                                                     ` Bernd Edlinger
2020-03-06 17:16                                                       ` Eric W. Biederman
2020-03-05 21:16                                               ` [PATCH 2/2] exec: Add a exec_update_mutex to replace cred_guard_mutex Eric W. Biederman
2020-03-05 21:51                                                 ` Bernd Edlinger
2020-03-06  5:17                                                   ` Eric W. Biederman
2020-03-06 11:46                                                     ` Bernd Edlinger
2020-03-06 21:18                                                       ` Eric W. Biederman
2020-03-06 19:16                                                     ` Bernd Edlinger
2020-03-06 21:58                                                       ` Eric W. Biederman
2020-03-06 22:29                                                         ` Eric W. Biederman
2020-03-07  1:03                                                           ` Eric W. Biederman
2020-03-08 12:58                                                             ` [PATCH] exec: make de_thread alloc new signal struct earlier Bernd Edlinger
2020-03-08 18:12                                                               ` Eric W. Biederman
2020-03-05 22:31                                               ` [PATCH 0/2] Infrastructure to allow fixing exec deadlocks Bernd Edlinger
2020-03-06  5:06                                                 ` Eric W. Biederman
2020-03-08 21:34                                               ` [PATCH 0/5] " Eric W. Biederman
2020-03-08 21:35                                                 ` [PATCH v2 1/5] exec: Only compute current once in flush_old_exec Eric W. Biederman
2020-03-09 13:56                                                   ` Bernd Edlinger
2020-03-09 17:34                                                     ` Eric W. Biederman
2020-03-09 17:56                                                       ` Bernd Edlinger
2020-03-09 19:27                                                         ` Bernd Edlinger
2020-03-10 20:17                                                   ` Kees Cook
2020-03-10 21:12                                                   ` Christian Brauner
2020-03-08 21:36                                                 ` [PATCH v2 2/5] exec: Factor unshare_sighand out of de_thread and call it separately Eric W. Biederman
2020-03-09 19:28                                                   ` Bernd Edlinger
2020-03-10 20:29                                                   ` Kees Cook
2020-03-10 20:34                                                     ` Bernd Edlinger
2020-03-10 20:57                                                       ` Kees Cook
2020-03-10 21:21                                                   ` Christian Brauner
2020-03-08 21:36                                                 ` [PATCH v2 3/5] exec: Move cleanup of posix timers on exec out of de_thread Eric W. Biederman
2020-03-09 19:30                                                   ` Bernd Edlinger
2020-03-09 19:59                                                   ` Christian Brauner
2020-03-09 20:06                                                     ` Eric W. Biederman
2020-03-09 20:17                                                       ` Christian Brauner
2020-03-09 20:48                                                         ` Eric W. Biederman
2020-03-10  8:55                                                           ` Christian Brauner
2020-03-10 18:52                                                             ` [PATCH] pidfd: Stop taking cred_guard_mutex Eric W. Biederman
2020-03-10 19:15                                                               ` Christian Brauner
2020-03-10 19:16                                                               ` Jann Horn
2020-03-10 19:27                                                                 ` Eric W. Biederman
2020-03-10 20:00                                                                   ` Jann Horn
2020-03-10 20:10                                                                     ` Jann Horn
2020-03-10 20:22                                                                       ` Bernd Edlinger
2020-03-11  6:11                                                                         ` Bernd Edlinger
2020-03-11 14:56                                                                           ` Jann Horn
2020-03-10 20:57                                                                       ` Eric W. Biederman
2020-03-10 21:29                                                                         ` Christian Brauner
2020-03-11 18:49                                                                         ` Kees Cook
2020-03-14  9:12                                                                           ` [PATCH] pidfd: Use new infrastructure to fix deadlocks in execve Bernd Edlinger
2020-03-10 20:16                                                           ` [PATCH v2 3/5] exec: Move cleanup of posix timers on exec out of de_thread Kees Cook
2020-03-10 20:31                                                   ` Kees Cook
2020-03-10 20:57                                                   ` Jann Horn
2020-03-10 21:05                                                     ` Eric W. Biederman
2020-03-10 21:22                                                   ` Christian Brauner
2020-03-08 21:38                                                 ` [PATCH v2 4/5] exec: Move exec_mmap right after de_thread in flush_old_exec Eric W. Biederman
2020-03-09 19:34                                                   ` Bernd Edlinger
2020-03-09 19:45                                                     ` Eric W. Biederman
2020-03-09 19:52                                                       ` Bernd Edlinger
2020-03-09 19:58                                                         ` Eric W. Biederman
2020-03-09 20:03                                                           ` Bernd Edlinger
2020-03-09 20:35                                                             ` Eric W. Biederman
2020-03-10 20:44                                                   ` Kees Cook
2020-03-10 21:20                                                     ` Eric W. Biederman
2020-03-10 20:47                                                   ` Kees Cook
2020-03-10 21:09                                                     ` Eric W. Biederman
2020-03-08 21:38                                                 ` [PATCH v2 5/5] exec: Add a exec_update_mutex to replace cred_guard_mutex Eric W. Biederman
2020-03-09 13:45                                                   ` Bernd Edlinger
2020-03-09 17:40                                                     ` Eric W. Biederman
2020-03-09 18:01                                                       ` Bernd Edlinger
2020-03-09 18:10                                                         ` Eric W. Biederman
2020-03-09 18:24                                                           ` Eric W. Biederman
2020-03-09 18:36                                                             ` Eric W. Biederman
2020-03-09 18:47                                                               ` Bernd Edlinger
2020-03-09 19:02                                                                 ` Eric W. Biederman
2020-03-09 19:24                                                                   ` Bernd Edlinger
2020-03-09 19:35                                                                     ` Eric W. Biederman
2020-03-09 19:39                                                                     ` Eric W. Biederman
2020-03-10 13:43                                                                       ` [PATCH 0/4] Use new infrastructure to fix deadlocks in execve Bernd Edlinger
2020-03-10 15:35                                                                         ` Eric W. Biederman
2020-03-10 17:44                                                                           ` [PATCH 0/4] Use new infrastructure in more simple cases Bernd Edlinger
2020-03-10 17:45                                                                           ` [PATCH 1/4] kernel/kcmp.c: Use new infrastructure to fix deadlocks in execve Bernd Edlinger
2020-03-10 19:01                                                                             ` Eric W. Biederman
2020-03-10 19:42                                                                               ` Bernd Edlinger
2020-03-10 17:45                                                                           ` [PATCH 2/4] proc: " Bernd Edlinger
2020-03-11 18:59                                                                             ` Kees Cook
2020-03-11 19:10                                                                             ` Kees Cook
2020-03-11 19:38                                                                               ` Bernd Edlinger
2020-03-10 17:45                                                                           ` [PATCH 3/4] proc: io_accounting: " Bernd Edlinger
2020-03-10 19:06                                                                             ` Eric W. Biederman [this message]
2020-03-10 20:19                                                                               ` Bernd Edlinger
2020-03-10 21:25                                                                                 ` Eric W. Biederman
2020-03-11 19:08                                                                             ` Kees Cook
2020-03-11 19:48                                                                               ` Bernd Edlinger
2020-03-11 19:48                                                                               ` Eric W. Biederman
2020-03-10 17:45                                                                           ` [PATCH 4/4] perf: " Bernd Edlinger
2020-03-10 13:43                                                                       ` [PATCH 1/4] exec: Fix a deadlock in ptrace Bernd Edlinger
2020-03-10 15:13                                                                         ` Eric W. Biederman
2020-03-10 15:17                                                                           ` Bernd Edlinger
2020-03-10 21:00                                                                         ` Kees Cook
2020-03-10 13:44                                                                       ` [PATCH 2/4] selftests/ptrace: add test cases for dead-locks Bernd Edlinger
2020-03-10 21:36                                                                         ` Kees Cook
2020-03-10 22:41                                                                         ` Dmitry V. Levin
2020-03-10 13:44                                                                       ` [PATCH 3/4] mm: docs: Fix a comment in process_vm_rw_core Bernd Edlinger
2020-03-11 18:53                                                                         ` Kees Cook
2020-03-10 13:44                                                                       ` [PATCH 4/4] kernel: doc: remove outdated comment cred.c Bernd Edlinger
2020-03-11 18:54                                                                         ` Kees Cook
2020-03-09 19:33                                                                   ` [PATCH v2 5/5] exec: Add a exec_update_mutex to replace cred_guard_mutex Dmitry V. Levin
2020-03-09 19:42                                                                     ` Eric W. Biederman
2020-03-10 20:55                                                                   ` Kees Cook
2020-03-10 21:02                                                                     ` Eric W. Biederman
2020-03-10 21:21                                                   ` Jann Horn
2020-03-10 21:30                                                     ` Eric W. Biederman
2020-03-10 23:21                                                       ` Jann Horn
2020-03-11  0:15                                                         ` Eric W. Biederman
2020-03-11  6:33                                                           ` Bernd Edlinger
2020-03-11 16:29                                                             ` Eric W. Biederman
2020-03-11 13:18                                                   ` Qian Cai
2020-03-12 10:27                                                   ` Kirill Tkhai
2020-03-12 12:24                                                     ` Eric W. Biederman
2020-03-12 13:45                                                       ` Kirill Tkhai
2020-03-12 14:38                                                         ` Eric W. Biederman
2020-03-12 15:23                                                           ` Kirill Tkhai
2020-03-13  1:05                                                           ` Bernd Edlinger
2020-03-13  9:13                                                             ` Kirill Tkhai
2020-03-14  9:11                                                               ` [PATCH v3 " Bernd Edlinger
2020-03-17  8:56                                                                 ` Kirill Tkhai
2020-03-17 21:53                                                                   ` Bernd Edlinger
2020-03-18 12:22                                                                     ` Kirill Tkhai
2020-03-18 20:06                                                                       ` Bernd Edlinger
2020-03-19  7:13                                                                         ` Kirill Tkhai
2020-03-19  7:19                                                                           ` Bernd Edlinger
2020-03-19  9:11                                                                           ` [PATCH v4 3/5] " Bernd Edlinger
2020-03-19  9:13                                                                             ` Bernd Edlinger
2020-03-19  9:19                                                                               ` Greg Kroah-Hartman
2020-03-19  9:20                                                                                 ` Bernd Edlinger
2020-03-21 22:53                                                                                 ` Bernd Edlinger
2020-03-14  9:12                                                               ` [PATCH 0/2] exec: Fix dead-lock in de_thread with ptrace_attach Bernd Edlinger
2020-03-14  9:12                                                               ` [PATCH 1/2] " Bernd Edlinger
2020-03-14  9:13                                                               ` [PATCH 2/2] doc: Update documentation of ->exec_*_mutex Bernd Edlinger
2020-03-14  9:57                                                               ` [PATCH v2 5/5] exec: Add a exec_update_mutex to replace cred_guard_mutex Bernd Edlinger
2020-03-14 10:02                                                                 ` Bernd Edlinger
2020-03-17  8:58                                                                   ` Kirill Tkhai
2020-03-09 13:58                                                 ` [PATCH 0/5] Infrastructure to allow fixing exec deadlocks Bernd Edlinger
2020-03-03 16:50                                 ` [PATCHv5] exec: Fix a deadlock in ptrace Christian Brauner
2020-03-02 12:28         ` [PATCHv2] " Oleg Nesterov
2020-03-02 15:56           ` Bernd Edlinger
2020-03-02  7:47       ` [PATCH] " Christian Brauner
2020-03-02  7:48         ` Christian Brauner

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=874kuwvxkz.fsf@x220.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=aarcange@redhat.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=avagin@gmail.com \
    --cc=bernd.edlinger@hotmail.de \
    --cc=bigeasy@linutronix.de \
    --cc=christian.brauner@ubuntu.com \
    --cc=christian@kellner.me \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=david@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=duyuyang@gmail.com \
    --cc=frederic@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jamorris@linux.microsoft.com \
    --cc=jannh@google.com \
    --cc=jgg@ziepe.ca \
    --cc=keescook@chromium.org \
    --cc=ldv@altlinux.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=shakeelb@google.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git