Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1539569

Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr

From Oleg Nesterov <oleg@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr
Date 2016-12-09 18:30 +0100
Message-ID <sMx9E-64e-13@gated-at.bofh.it> (permalink)
References <sMpOO-1yx-11@gated-at.bofh.it> <sMpOO-1yx-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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);
 

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] sched/pid fix use-after free in task_tgid_vnr EunTaik Lee <eun.taik.lee@samsung.com> - 2016-12-09 10:40 +0100
  Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr Oleg Nesterov <oleg@redhat.com> - 2016-12-09 18:30 +0100
    Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr ebiederm@xmission.com (Eric W. Biederman) - 2016-12-09 23:30 +0100
      Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr Oleg Nesterov <oleg@redhat.com> - 2016-12-12 14:50 +0100
        Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr ebiederm@xmission.com (Eric W. Biederman) - 2016-12-12 20:20 +0100
          Re: [PATCH] sched/pid fix use-after free in task_tgid_vnr Oleg Nesterov <oleg@redhat.com> - 2016-12-13 17:10 +0100

csiph-web