All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: x86@kernel.org
Cc: linux-kernel@vger.kernel.org, Brian Gerst <brgerst@gmail.com>,
	Borislav Petkov <bp@alien8.de>, Jann Horn <jann@thejh.net>,
	Linux API <linux-api@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	Tycho Andersen <tycho.andersen@canonical.com>,
	Andy Lutomirski <luto@kernel.org>
Subject: [PATCH 3/3] mm: Change vm_is_stack_for_task() to vm_is_stack_for_current()
Date: Fri, 30 Sep 2016 10:58:58 -0700	[thread overview]
Message-ID: <4c3f68f426e6c061ca98b4fc7ef85ffbb0a25b0c.1475257877.git.luto@kernel.org> (raw)
In-Reply-To: <cover.1475257877.git.luto@kernel.org>
In-Reply-To: <cover.1475257877.git.luto@kernel.org>

Asking for a non-current task's stack can't be done without races
unless the task is frozen in kernel mode.  As far as I know,
vm_is_stack_for_task() never had a safe non-current use case.

The __unused annotation is because some KSTK_ESP implementations
ignore their parameter, which IMO is further justification for this
patch.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 include/linux/mm.h       | 2 +-
 mm/util.c                | 4 +++-
 security/selinux/hooks.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index ef815b9cd426..c365cf49f54a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1403,7 +1403,7 @@ static inline int stack_guard_page_end(struct vm_area_struct *vma,
 		!vma_growsup(vma->vm_next, addr);
 }
 
-int vma_is_stack_for_task(struct vm_area_struct *vma, struct task_struct *t);
+int vma_is_stack_for_current(struct vm_area_struct *vma);
 
 extern unsigned long move_page_tables(struct vm_area_struct *vma,
 		unsigned long old_addr, struct vm_area_struct *new_vma,
diff --git a/mm/util.c b/mm/util.c
index 662cddf914af..c174e8921995 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -230,8 +230,10 @@ void __vma_link_list(struct mm_struct *mm, struct vm_area_struct *vma,
 }
 
 /* Check if the vma is being used as a stack by this task */
-int vma_is_stack_for_task(struct vm_area_struct *vma, struct task_struct *t)
+int vma_is_stack_for_current(struct vm_area_struct *vma)
 {
+	struct task_struct * __maybe_unused t = current;
+
 	return (vma->vm_start <= KSTK_ESP(t) && vma->vm_end >= KSTK_ESP(t));
 }
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 13185a6c266a..3a40135bde5e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3505,7 +3505,7 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
 		} else if (!vma->vm_file &&
 			   ((vma->vm_start <= vma->vm_mm->start_stack &&
 			     vma->vm_end >= vma->vm_mm->start_stack) ||
-			    vma_is_stack_for_task(vma, current))) {
+			    vma_is_stack_for_current(vma))) {
 			rc = current_has_perm(current, PROCESS__EXECSTACK);
 		} else if (vma->vm_file && vma->anon_vma) {
 			/*
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>,
	Jann Horn <jann-XZ1E9jl8jIdeoWH0uzbU5w@public.gmane.org>,
	Linux API <linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linus Torvalds
	<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	Tycho Andersen
	<tycho.andersen-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>,
	Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: [PATCH 3/3] mm: Change vm_is_stack_for_task() to vm_is_stack_for_current()
Date: Fri, 30 Sep 2016 10:58:58 -0700	[thread overview]
Message-ID: <4c3f68f426e6c061ca98b4fc7ef85ffbb0a25b0c.1475257877.git.luto@kernel.org> (raw)
In-Reply-To: <cover.1475257877.git.luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
In-Reply-To: <cover.1475257877.git.luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Asking for a non-current task's stack can't be done without races
unless the task is frozen in kernel mode.  As far as I know,
vm_is_stack_for_task() never had a safe non-current use case.

The __unused annotation is because some KSTK_ESP implementations
ignore their parameter, which IMO is further justification for this
patch.

Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/linux/mm.h       | 2 +-
 mm/util.c                | 4 +++-
 security/selinux/hooks.c | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index ef815b9cd426..c365cf49f54a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1403,7 +1403,7 @@ static inline int stack_guard_page_end(struct vm_area_struct *vma,
 		!vma_growsup(vma->vm_next, addr);
 }
 
-int vma_is_stack_for_task(struct vm_area_struct *vma, struct task_struct *t);
+int vma_is_stack_for_current(struct vm_area_struct *vma);
 
 extern unsigned long move_page_tables(struct vm_area_struct *vma,
 		unsigned long old_addr, struct vm_area_struct *new_vma,
diff --git a/mm/util.c b/mm/util.c
index 662cddf914af..c174e8921995 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -230,8 +230,10 @@ void __vma_link_list(struct mm_struct *mm, struct vm_area_struct *vma,
 }
 
 /* Check if the vma is being used as a stack by this task */
-int vma_is_stack_for_task(struct vm_area_struct *vma, struct task_struct *t)
+int vma_is_stack_for_current(struct vm_area_struct *vma)
 {
+	struct task_struct * __maybe_unused t = current;
+
 	return (vma->vm_start <= KSTK_ESP(t) && vma->vm_end >= KSTK_ESP(t));
 }
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 13185a6c266a..3a40135bde5e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3505,7 +3505,7 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
 		} else if (!vma->vm_file &&
 			   ((vma->vm_start <= vma->vm_mm->start_stack &&
 			     vma->vm_end >= vma->vm_mm->start_stack) ||
-			    vma_is_stack_for_task(vma, current))) {
+			    vma_is_stack_for_current(vma))) {
 			rc = current_has_perm(current, PROCESS__EXECSTACK);
 		} else if (vma->vm_file && vma->anon_vma) {
 			/*
-- 
2.7.4

  parent reply	other threads:[~2016-09-30 17:59 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-30 17:58 [PATCH 0/3] ABI CHANGE!!! Remove questionable remote SP reads Andy Lutomirski
2016-09-30 17:58 ` Andy Lutomirski
2016-09-30 17:58 ` [PATCH 1/3] proc: Stop reporting eip and esp in /proc/PID/stat Andy Lutomirski
2016-09-30 17:58   ` Andy Lutomirski
2016-09-30 18:56   ` Jann Horn
2016-09-30 18:56     ` Jann Horn
2016-10-01  2:01     ` Andy Lutomirski
2016-10-01  2:01       ` Andy Lutomirski
2016-10-01  4:22       ` Linus Torvalds
2016-10-01  4:22         ` Linus Torvalds
2016-10-01 10:37       ` Jann Horn
2016-10-01 10:37         ` Jann Horn
2016-10-14 18:25         ` Andy Lutomirski
2016-10-14 18:25           ` Andy Lutomirski
2016-10-14 20:01           ` Tycho Andersen
2016-10-20 11:13   ` [tip:mm/urgent] fs/proc: " tip-bot for Andy Lutomirski
2016-11-01 14:36   ` [4.9-rc3] BUG: unable to handle kernel paging request at ffffc900144dfc60 Tetsuo Handa
2016-11-01 23:47     ` Linus Torvalds
2016-11-02 10:50       ` Tetsuo Handa
2016-11-02 14:05         ` Andy Lutomirski
2016-11-02 14:05           ` Andy Lutomirski
2016-11-02 14:54         ` Linus Torvalds
2016-11-03  6:32           ` Ingo Molnar
2016-11-03  7:09         ` [tip:sched/urgent] sched/core: Fix oops in sched_show_task() tip-bot for Tetsuo Handa
2016-11-03  7:10       ` [tip:sched/urgent] sched/core: Remove pointless printout " tip-bot for Linus Torvalds
2016-09-30 17:58 ` [PATCH 2/3] proc: Stop trying to report thread stacks Andy Lutomirski
2016-10-20 11:13   ` [tip:mm/urgent] fs/proc: " tip-bot for Andy Lutomirski
2016-09-30 17:58 ` Andy Lutomirski [this message]
2016-09-30 17:58   ` [PATCH 3/3] mm: Change vm_is_stack_for_task() to vm_is_stack_for_current() Andy Lutomirski
2016-10-20 11:14   ` [tip:mm/urgent] " tip-bot for Andy Lutomirski
2016-10-03 23:08 ` [PATCH 0/3] ABI CHANGE!!! Remove questionable remote SP reads Andy Lutomirski
2016-10-03 23:08   ` Andy Lutomirski
2016-10-03 23:17   ` Linus Torvalds
2016-10-03 23:17     ` Linus Torvalds
2016-10-04  7:06     ` Raymond Jennings
2016-10-04  7:06       ` Raymond Jennings
2016-10-14 18:26     ` Andy Lutomirski
2016-10-14 18:26       ` Andy Lutomirski

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=4c3f68f426e6c061ca98b4fc7ef85ffbb0a25b0c.1475257877.git.luto@kernel.org \
    --to=luto@kernel.org \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=jann@thejh.net \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tycho.andersen@canonical.com \
    --cc=x86@kernel.org \
    /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.