All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>
Cc: James Morris <jmorris@namei.org>, Ingo Molnar <mingo@elte.hu>,
	Chris Wright <chrisw@sous-sol.org>,
	Roland McGrath <roland@redhat.com>,
	linux-kernel@vger.kernel.org, Al Viro <viro@ZenIV.linux.org.uk>,
	linux-security-module@vger.kernel.org
Subject: [PATCH 1/1] mm_for_maps: simplify, use ptrace_may_access()
Date: Tue, 23 Jun 2009 21:25:32 +0200	[thread overview]
Message-ID: <20090623192532.GB27853@redhat.com> (raw)
In-Reply-To: <20090623192451.GA27853@redhat.com>

It would be nice to kill __ptrace_may_access(). It requires task_lock(),
but this lock is only needed to read mm->flags in the middle.

Convert mm_for_maps() to use ptrace_may_access(), this also simplifies
the code a little bit.

Also, we do not need to take ->mmap_sem in advance. In fact I think
mm_for_maps() should not play with ->mmap_sem at all, the caller should
take this lock.

With or without this patch, without ->cred_guard_mutex held we can race
with exec() and get the new ->mm but check old creds.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>

--- WAIT/fs/proc/base.c~1_MM_FOR_MAPS	2009-06-17 14:11:26.000000000 +0200
+++ WAIT/fs/proc/base.c	2009-06-23 20:16:44.000000000 +0200
@@ -237,20 +237,19 @@ struct mm_struct *mm_for_maps(struct tas
 	struct mm_struct *mm = get_task_mm(task);
 	if (!mm)
 		return NULL;
+	if (mm != current->mm) {
+		/*
+		 * task->mm can be changed before security check,
+		 * in that case we must notice the change after.
+		 */
+		if (!ptrace_may_access(task, PTRACE_MODE_READ) ||
+		    mm != task->mm) {
+			mmput(mm);
+			return NULL;
+		}
+	}
 	down_read(&mm->mmap_sem);
-	task_lock(task);
-	if (task->mm != mm)
-		goto out;
-	if (task->mm != current->mm &&
-	    __ptrace_may_access(task, PTRACE_MODE_READ) < 0)
-		goto out;
-	task_unlock(task);
 	return mm;
-out:
-	task_unlock(task);
-	up_read(&mm->mmap_sem);
-	mmput(mm);
-	return NULL;
 }
 
 static int proc_pid_cmdline(struct task_struct *task, char * buffer)


  reply	other threads:[~2009-06-23 22:40 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-05 22:47 [PATCH 3/3] ptrace: do not use task_lock() for attach Oleg Nesterov
2009-05-06  2:08 ` Roland McGrath
2009-05-06  8:00 ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-06 20:32   ` Roland McGrath
2009-05-06 20:47     ` Christoph Hellwig
2009-05-06 21:09       ` Roland McGrath
2009-05-07  8:19       ` Ingo Molnar
2009-05-07  8:17     ` Ingo Molnar
2009-05-06 23:53   ` Oleg Nesterov
2009-05-07  0:21     ` Roland McGrath
2009-05-07  6:36       ` Oleg Nesterov
2009-05-07  8:20         ` Ingo Molnar
2009-05-07  8:31           ` Oleg Nesterov
2009-05-07  8:38             ` Ingo Molnar
2009-05-07  8:49               ` [patch] security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07  9:19                 ` Oleg Nesterov
2009-05-07  9:27                   ` Ingo Molnar
2009-05-07  8:57               ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Chris Wright
2009-05-07  9:04                 ` Ingo Molnar
2009-05-07  9:20                   ` Chris Wright
2009-05-07  9:54                     ` James Morris
2009-05-07 10:20                       ` your mail Ingo Molnar
2009-05-07 11:37                         ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07 14:17                           ` Ingo Molnar
2009-06-23 14:14                           ` Oleg Nesterov
2009-06-23 17:49                             ` Christoph Hellwig
2009-06-23 19:24                               ` [PATCH 0/1] mm_for_maps: simplify, use ptrace_may_access() Oleg Nesterov
2009-06-23 19:25                                 ` Oleg Nesterov [this message]
2009-06-24  3:06                                   ` [PATCH 1/1] " Serge E. Hallyn
2009-06-24 14:21                                     ` James Morris
2009-06-24  9:25                                   ` Roland McGrath
2009-06-24 14:37                                     ` Oleg Nesterov
2009-06-24  1:08                             ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-08  3:27                         ` your mail Casey Schaufler
2009-06-24 14:19                       ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07  9:31                   ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-07  9:49                     ` [patch 1/2] ptrace, security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07 18:47                       ` Roland McGrath
2009-05-07 19:55                       ` Andrew Morton
2009-05-11 13:39                         ` Ingo Molnar
2009-05-11 18:51                           ` Andrew Morton
2009-05-15  1:10                           ` Américo Wang
2009-05-15 19:34                             ` Ingo Molnar
2009-05-07  9:50                     ` [patch 2/2] ptrace: turn ptrace_access_check() into a retval function Ingo Molnar
2009-05-07 18:47                       ` Roland McGrath
2009-05-06 22:46 ` [PATCH 3/3] ptrace: do not use task_lock() for attach Chris Wright
2009-05-06 23:13   ` Oleg Nesterov
2009-05-06 23:27     ` Chris Wright
2009-05-06 23:48       ` James Morris
2009-05-07  1:17         ` Roland McGrath
2009-05-08 12:18           ` David Howells

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=20090623192532.GB27853@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=chrisw@sous-sol.org \
    --cc=hch@infradead.org \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.