Commit 2dd8083f authored by Eric W. Biederman's avatar Eric W. Biederman
Browse files

posix-cpu-timers: Use pids not tasks in lookup 



The current posix-cpu-timer code uses pids when holding persistent
references in timers.  However the lookups from clock_id_t still return
tasks that need to be converted into pids for use.

This results in usage being pid->task->pid and that can race with
release_task and de_thread.  This can lead to some not wrong but
surprising results.  Surprising enough that Oleg and I both thought
there were some bugs in the code for a while.

This set of changes modifies the code to just lookup, verify, and return
pids from the clockid_t lookups to remove those potentialy troublesome
races.

Eric W. Biederman (3):
      posix-cpu-timers: Extend rcu_read_lock removing task_struct references
      posix-cpu-timers: Replace cpu_timer_pid_type with clock_pid_type
      posix-cpu-timers: Replace __get_task_for_clock with pid_for_clock

 kernel/time/posix-cpu-timers.c | 102 ++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 57 deletions(-)

Suggested-by: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
parents 1dd694a1 96498773

kernel/time/posix-cpu-timers.c

+45 −57
Original line number Diff line number Diff line
@@ -47,80 +47,65 @@ void update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new) 
/*

 * Functions for validating access to tasks.

 */

static struct task_struct *lookup_task(const pid_t pid, bool thread,

				       bool gettime)

static struct pid *pid_for_clock(const clockid_t clock, bool gettime)

{

	struct task_struct *p;

	const bool thread = !!CPUCLOCK_PERTHREAD(clock);

	const pid_t upid = CPUCLOCK_PID(clock);

	struct pid *pid;



	if (CPUCLOCK_WHICH(clock) >= CPUCLOCK_MAX)

		return NULL;



	/*

	 * If the encoded PID is 0, then the timer is targeted at current

	 * or the process to which current belongs.

	 */

	if (!pid)

		return thread ? current : current->group_leader;

	if (upid == 0)

		return thread ? task_pid(current) : task_tgid(current);



	p = find_task_by_vpid(pid);

	if (!p)

		return p;

	pid = find_vpid(upid);

	if (!pid)

		return NULL;



	if (thread)

		return same_thread_group(p, current) ? p : NULL;

	if (thread) {

		struct task_struct *tsk = pid_task(pid, PIDTYPE_PID);

		return (tsk && same_thread_group(tsk, current)) ? pid : NULL;

	}



	/*

	 * For clock_gettime(PROCESS) the task does not need to be

	 * the actual group leader. task->signal gives

	 * access to the group's clock.

	 * For clock_gettime(PROCESS) allow finding the process by

	 * with the pid of the current task.  The code needs the tgid

	 * of the process so that pid_task(pid, PIDTYPE_TGID) can be

	 * used to find the process.

	 */

	if (gettime && (p == current))

		return p;

	if (gettime && (pid == task_pid(current)))

		return task_tgid(current);



	/*

	 * For processes require that p is group leader.

	 * For processes require that pid identifies a process.

	 */

	return thread_group_leader(p) ? p : NULL;

	return pid_has_task(pid, PIDTYPE_TGID) ? pid : NULL;

}



static struct task_struct *__get_task_for_clock(const clockid_t clock,

						bool getref, bool gettime)

static inline int validate_clock_permissions(const clockid_t clock)

{

	const bool thread = !!CPUCLOCK_PERTHREAD(clock);

	const pid_t pid = CPUCLOCK_PID(clock);

	struct task_struct *p;



	if (CPUCLOCK_WHICH(clock) >= CPUCLOCK_MAX)

		return NULL;

	int ret;



	rcu_read_lock();

	p = lookup_task(pid, thread, gettime);

	if (p && getref)

		get_task_struct(p);

	ret = pid_for_clock(clock, false) ? 0 : -EINVAL;

	rcu_read_unlock();

	return p;

}



static inline struct task_struct *get_task_for_clock(const clockid_t clock)

{

	return __get_task_for_clock(clock, true, false);

}



static inline struct task_struct *get_task_for_clock_get(const clockid_t clock)

{

	return __get_task_for_clock(clock, true, true);

}



static inline int validate_clock_permissions(const clockid_t clock)

{

	return __get_task_for_clock(clock, false, false) ? 0 : -EINVAL;

	return ret;

}



static inline enum pid_type cpu_timer_pid_type(struct k_itimer *timer)

static inline enum pid_type clock_pid_type(const clockid_t clock)

{

	return CPUCLOCK_PERTHREAD(timer->it_clock) ? PIDTYPE_PID : PIDTYPE_TGID;

	return CPUCLOCK_PERTHREAD(clock) ? PIDTYPE_PID : PIDTYPE_TGID;

}



static inline struct task_struct *cpu_timer_task_rcu(struct k_itimer *timer)

{

	return pid_task(timer->it.cpu.pid, cpu_timer_pid_type(timer));

	return pid_task(timer->it.cpu.pid, clock_pid_type(timer->it_clock));

}



/*
@@ -368,15 +353,18 @@ static int posix_cpu_clock_get(const clockid_t clock, struct timespec64 *tp) 
	struct task_struct *tsk;

	u64 t;



	tsk = get_task_for_clock_get(clock);

	if (!tsk)

	rcu_read_lock();

	tsk = pid_task(pid_for_clock(clock, true), clock_pid_type(clock));

	if (!tsk) {

		rcu_read_unlock();

		return -EINVAL;

	}



	if (CPUCLOCK_PERTHREAD(clock))

		t = cpu_clock_sample(clkid, tsk);

	else

		t = cpu_clock_sample_group(clkid, tsk, false);

	put_task_struct(tsk);

	rcu_read_unlock();



	*tp = ns_to_timespec64(t);

	return 0;
@@ -389,19 +377,19 @@ static int posix_cpu_clock_get(const clockid_t clock, struct timespec64 *tp) 
 */

static int posix_cpu_timer_create(struct k_itimer *new_timer)

{

	struct task_struct *p = get_task_for_clock(new_timer->it_clock);

	struct pid *pid;



	if (!p)

	rcu_read_lock();

	pid = pid_for_clock(new_timer->it_clock, false);

	if (!pid) {

		rcu_read_unlock();

		return -EINVAL;

	}



	new_timer->kclock = &clock_posix_cpu;

	timerqueue_init(&new_timer->it.cpu.node);

	new_timer->it.cpu.pid = get_task_pid(p, cpu_timer_pid_type(new_timer));

	/*

	 * get_task_for_clock() took a reference on @p. Drop it as the timer

	 * holds a reference on the pid of @p.

	 */

	put_task_struct(p);

	new_timer->it.cpu.pid = get_pid(pid);

	rcu_read_unlock();

	return 0;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE