From: Oleg Nesterov <oleg@redhat.com>
To: EunTaik Lee <eun.taik.lee@samsung.com>,
"Eric W. Biederman" <ebiederm@xmission.com>
Cc: "mingo@redhat.com" <mingo@redhat.com>,
"peterz@infradead.org" <peterz@infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr
Date: Fri, 9 Dec 2016 18:21:15 +0100 [thread overview]
Message-ID: <20161209172114.GA25742@redhat.com> (raw)
In-Reply-To: <20161209093351epcms1p418673c3cdec7d4c3e81b5df131173c57@epcms1p4>
On 12/09, EunTaik Lee wrote:
>
> There is a use-after-free case with below call stack.
>
> pid_nr_ns+0x10/0x38
> cgroup_pidlist_start+0x144/0x400
> cgroup_seqfile_start+0x1c/0x24
> kernfs_seq_start+0x54/0x90
> seq_read+0x15c/0x3a8
> kernfs_fop_read+0x38/0x160
> __vfs_read+0x28/0xc8
> vfs_read+0x84/0xfc
This reminds about perf_event_pid() which is equally buggy...
> static inline pid_t task_tgid_vnr(struct task_struct *tsk)
> {
> - return pid_vnr(task_tgid(tsk));
> + pid_t pid = 0;
> +
> + rcu_read_lock();
> + if (pid_alive(tsk))
> + pid = pid_vnr(task_tgid(tsk));
> + rcu_read_unlock();
> +
> + return pid;
> }
Eric, EunTaik, what do you think about the patch below?
I can't decide whether it is too ugly or not, but it would be nice
to avoid the code duplication.
Oleg.
--- x/include/linux/pid.h
+++ x/include/linux/pid.h
@@ -8,7 +8,8 @@ enum pid_type
PIDTYPE_PID,
PIDTYPE_PGID,
PIDTYPE_SID,
- PIDTYPE_MAX
+ PIDTYPE_MAX,
+ PIDTYPE_TGID /* do not use */
};
/*
--- x/kernel/pid.c
+++ x/kernel/pid.c
@@ -526,8 +526,11 @@ pid_t __task_pid_nr_ns(struct task_struc
if (!ns)
ns = task_active_pid_ns(current);
if (likely(pid_alive(task))) {
- if (type != PIDTYPE_PID)
+ if (type != PIDTYPE_PID) {
+ if (type == PIDTYPE_TGID)
+ type = PIDTYPE_PID;
task = task->group_leader;
+ }
nr = pid_nr_ns(rcu_dereference(task->pids[type].pid), ns);
}
rcu_read_unlock();
@@ -538,7 +541,7 @@ EXPORT_SYMBOL(__task_pid_nr_ns);
pid_t task_tgid_nr_ns(struct task_struct *tsk, struct pid_namespace *ns)
{
- return pid_nr_ns(task_tgid(tsk), ns);
+ return __task_pid_nr_ns(tsk, PIDTYPE_TGID, ns);
}
EXPORT_SYMBOL(task_tgid_nr_ns);
next prev parent reply other threads:[~2016-12-09 17:21 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20161209093351epcms1p418673c3cdec7d4c3e81b5df131173c57@epcms1p4>
2016-12-09 9:33 ` [PATCH] sched/pid fix use-after free in task_tgid_vnr EunTaik Lee
2016-12-09 17:21 ` Oleg Nesterov [this message]
2016-12-09 22:21 ` Eric W. Biederman
2016-12-12 13:46 ` Oleg Nesterov
2016-12-12 19:10 ` Eric W. Biederman
2016-12-13 16:03 ` Oleg Nesterov
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=20161209172114.GA25742@redhat.com \
--to=oleg@redhat.com \
--cc=ebiederm@xmission.com \
--cc=eun.taik.lee@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).