diff options
| author | Adrian Salido <salidoa@google.com> | 2017-03-03 12:06:36 +0100 |
|---|---|---|
| committer | Bertolin, PierreX <pierrex.bertolin@intel.com> | 2017-03-14 01:54:01 -0700 |
| commit | 1199dbf571d51510c7bf4d62d7afef2d67cccca0 (patch) | |
| tree | 0ce73018e5b4ad8b203a1aeb52900648ea7c26b6 | |
| parent | 105faac656d3c861ab7cfe39c666d7c2a4286870 (diff) | |
| download | x86-1199dbf571d51510c7bf4d62d7afef2d67cccca0.tar.gz | |
fs/proc/array.c: make safe access to group_leader
As mentioned in commit 52ee2dfdd4f51cf422ea6a96a0846dc94244aa37
("pids: refactor vnr/nr_ns helpers to make them safe"). *_nr_ns
helpers used to be buggy. The commit addresses most of the helpers but
is missing task_tgid_xxx()
Without this protection there is a possible use after free reported by
kasan instrumented kernel:
==================================================================
BUG: KASAN: use-after-free in task_tgid_nr_ns+0x2c/0x44 at addr ***
Read of size 8 by task cat/2472
CPU: 1 PID: 2472 Comm: cat Tainted: ****
Hardware name: Google Tegra210 Smaug Rev 1,3+ (DT)
Call trace:
[<ffffffc00020ad2c>] dump_backtrace+0x0/0x17c
[<ffffffc00020aec0>] show_stack+0x18/0x24
[<ffffffc0011573d0>] dump_stack+0x94/0x100
[<ffffffc0003c7dc0>] kasan_report+0x308/0x554
[<ffffffc0003c7518>] __asan_load8+0x20/0x7c
[<ffffffc00025a54c>] task_tgid_nr_ns+0x28/0x44
[<ffffffc00046951c>] proc_pid_status+0x444/0x1080
[<ffffffc000460f60>] proc_single_show+0x8c/0xdc
[<ffffffc0004081b0>] seq_read+0x2e8/0x6f0
[<ffffffc0003d1420>] vfs_read+0xd8/0x1e0
[<ffffffc0003d1b98>] SyS_read+0x68/0xd4
Accessing group_leader while holding rcu_lock and using the now safe
helpers introduced in the commit mentioned, this race condition is
addressed.
Bug: 31495866
Change-Id: I8dc530762759b29f662a14b5832b5801d1e75ae2
Signed-off-by: Adrian Salido <salidoa@google.com>
Signed-off-by: Louis, FabienX <fabienx.louis@intel.com>
Signed-off-by: Simon Dubray <simonx.dubray@intel.com>
Tracked-On: https://jira01.devtools.intel.com/browse/AW-4548
Reviewed-on: https://android.intel.com/571450
Reviewed-by: Tasayco Loarte, VictorX <victorx.tasayco.loarte@intel.com>
| -rw-r--r-- | fs/proc/array.c | 12 |
1 files changed, 7 insertions, 5 deletions
diff --git a/fs/proc/array.c b/fs/proc/array.c index b6c00ce0e29e..9bc0569e9c6f 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -147,18 +147,20 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns, int g; struct task_struct *tracer; const struct cred *cred; - pid_t ppid, tpid = 0, tgid, ngid; + pid_t ppid = 0, tpid = 0, tgid, ngid; unsigned int max_fds = 0; + struct task_struct *leader = NULL; rcu_read_lock(); - ppid = pid_alive(p) ? - task_tgid_nr_ns(rcu_dereference(p->real_parent), ns) : 0; + if (pid_alive(p)) { + ppid = task_tgid_nr_ns(rcu_dereference(p->real_parent), ns); + leader = p->group_leader; + } tracer = ptrace_parent(p); if (tracer) tpid = task_pid_nr_ns(tracer, ns); - - tgid = task_tgid_nr_ns(p, ns); + tgid = leader ? task_pid_nr_ns(leader, ns) : 0; ngid = task_numa_group_id(p); cred = get_task_cred(p); |