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
next prev 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: linkBe 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.