From: "zhangyi (F)" <yi.zhang@huawei.com>
To: <gregkh@linuxfoundation.org>
Cc: <linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
<luto@kernel.org>, <adobriyan@gmail.com>,
<akpm@linux-foundation.org>, <vbabka@suse.cz>,
<peterz@infradead.org>, <bigeasy@linutronix.de>,
<mhocko@suse.com>, <john.ogness@linutronix.de>,
<yi.zhang@huawei.com>, <nixiaoming@huawei.com>
Subject: [PATCH 4.4 6/7] proc: fix coredump vs read /proc/*/stat race
Date: Mon, 2 Dec 2019 16:35:18 +0800 [thread overview]
Message-ID: <20191202083519.23138-7-yi.zhang@huawei.com> (raw)
In-Reply-To: <20191202083519.23138-1-yi.zhang@huawei.com>
From: Alexey Dobriyan <adobriyan@gmail.com>
commit 8bb2ee192e482c5d500df9f2b1b26a560bd3026f upstream.
do_task_stat() accesses IP and SP of a task without bumping reference
count of a stack (which became an entity with independent lifetime at
some point).
Steps to reproduce:
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <unistd.h>
#include <sys/wait.h>
int main(void)
{
setrlimit(RLIMIT_CORE, &(struct rlimit){});
while (1) {
char buf[64];
char buf2[4096];
pid_t pid;
int fd;
pid = fork();
if (pid == 0) {
*(volatile int *)0 = 0;
}
snprintf(buf, sizeof(buf), "/proc/%u/stat", pid);
fd = open(buf, O_RDONLY);
read(fd, buf2, sizeof(buf2));
close(fd);
waitpid(pid, NULL, 0);
}
return 0;
}
BUG: unable to handle kernel paging request at 0000000000003fd8
IP: do_task_stat+0x8b4/0xaf0
PGD 800000003d73e067 P4D 800000003d73e067 PUD 3d558067 PMD 0
Oops: 0000 [#1] PREEMPT SMP PTI
CPU: 0 PID: 1417 Comm: a.out Not tainted 4.15.0-rc8-dirty #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc27 04/01/2014
RIP: 0010:do_task_stat+0x8b4/0xaf0
Call Trace:
proc_single_show+0x43/0x70
seq_read+0xe6/0x3b0
__vfs_read+0x1e/0x120
vfs_read+0x84/0x110
SyS_read+0x3d/0xa0
entry_SYSCALL_64_fastpath+0x13/0x6c
RIP: 0033:0x7f4d7928cba0
RSP: 002b:00007ffddb245158 EFLAGS: 00000246
Code: 03 b7 a0 01 00 00 4c 8b 4c 24 70 4c 8b 44 24 78 4c 89 74 24 18 e9 91 f9 ff ff f6 45 4d 02 0f 84 fd f7 ff ff 48 8b 45 40 48 89 ef <48> 8b 80 d8 3f 00 00 48 89 44 24 20 e8 9b 97 eb ff 48 89 44 24
RIP: do_task_stat+0x8b4/0xaf0 RSP: ffffc90000607cc8
CR2: 0000000000003fd8
John Ogness said: for my tests I added an else case to verify that the
race is hit and correctly mitigated.
Link: http://lkml.kernel.org/r/20180116175054.GA11513@avx2
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Reported-by: "Kohli, Gaurav" <gkohli@codeaurora.org>
Tested-by: John Ogness <john.ogness@linutronix.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
fs/proc/array.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index c3e1732bcaa5..42e33ea50d1c 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -435,8 +435,11 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
* safe because the task has stopped executing permanently.
*/
if (permitted && (task->flags & PF_DUMPCORE)) {
- eip = KSTK_EIP(task);
- esp = KSTK_ESP(task);
+ if (try_get_task_stack(task)) {
+ eip = KSTK_EIP(task);
+ esp = KSTK_ESP(task);
+ put_task_stack(task);
+ }
}
}
--
2.17.2
next prev parent reply other threads:[~2019-12-02 8:14 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-02 8:35 [PATCH 4.4 0/7] fs/proc: Stop reporting eip and esp in zhangyi (F)
2019-12-02 8:35 ` [PATCH 4.4 1/7] sched/core: Allow putting thread_info into task_struct zhangyi (F)
2019-12-02 8:35 ` [PATCH 4.4 2/7] sched/core: Add try_get_task_stack() and put_task_stack() zhangyi (F)
2019-12-02 8:35 ` [PATCH 4.4 3/7] sched/core, x86: Make struct thread_info arch specific again zhangyi (F)
2019-12-02 8:35 ` [PATCH 4.4 4/7] fs/proc: Stop reporting eip and esp in /proc/PID/stat zhangyi (F)
2019-12-02 8:35 ` [PATCH 4.4 5/7] fs/proc: Report eip/esp in /prod/PID/stat for coredumping zhangyi (F)
2019-12-02 8:35 ` zhangyi (F) [this message]
2019-12-02 8:35 ` [PATCH 4.4 7/7] fs/proc/array.c: allow reporting eip/esp for all coredumping threads zhangyi (F)
2019-12-12 17:15 ` [PATCH 4.4 0/7] fs/proc: Stop reporting eip and esp in Greg KH
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=20191202083519.23138-7-yi.zhang@huawei.com \
--to=yi.zhang@huawei.com \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=bigeasy@linutronix.de \
--cc=gregkh@linuxfoundation.org \
--cc=john.ogness@linutronix.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mhocko@suse.com \
--cc=nixiaoming@huawei.com \
--cc=peterz@infradead.org \
--cc=vbabka@suse.cz \
/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.