posix-cpu-timers: Store a reference to a pid not a task

posix cpu timers do not handle the death of a process well. This is most clearly seen when a multi-threaded process calls exec from a thread that is not the leader of the thread group. The posix cpu timer code continues to pin the old thread group leader and is unable to find the siglock from there. This results in posix_cpu_timer_del being unable to delete a timer, posix_cpu_timer_set being unable to set a timer. Further to compensate for the problems in posix_cpu_timer_del on a multi-threaded exec all timers that point at the multi-threaded task are stopped. The code for the timers fundamentally needs to check if the target process/thread is alive. This needs an extra level of indirection. This level of indirection is already available in struct pid. So replace cpu.task with cpu.pid to get the needed extra layer of indirection. In addition to handling things more cleanly this reduces the amount of memory a timer can pin when a process exits and then is reaped from a task_struct to the vastly smaller struct pid.

Eric W. Biederman committed 6 years ago

Commit 55e8c8e1 parent beb41d92 files · +56 −19Browse files

Unified · Split

2 files

include/linux/posix-timers.h +1 −1
kernel/time/posix-cpu-timers.c +55 −18

include/linux/posix-timers.h+1−1

@@ -69,7 +69,7 @@ static inline int clockid_to_fd(const clockid_t clk) · 68 unmodified lines
69	struct cpu_timer {	69	struct cpu_timer {
70	struct timerqueue_node node;	70	struct timerqueue_node node;
71	struct timerqueue_head *head;	71	struct timerqueue_head *head;
72	struct task_struct *task;	72	struct pid *pid;
73	struct list_head elist;	73	struct list_head elist;
74	int firing;	74	int firing;
75	};	75	};

kernel/time/posix-cpu-timers.c+55−18

 	return __get_task_for_clock(clock, false, false) ? 0 : -EINVAL;
 }
+static inline enum pid_type cpu_timer_pid_type(struct k_itimer *timer)
+{
+	return CPUCLOCK_PERTHREAD(timer->it_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));
+}
 /*
  * Update expiry time from increment, and increase overrun count,
  * given the current clock sample.
 	new_timer->kclock = &clock_posix_cpu;
 	timerqueue_init(&new_timer->it.cpu.node);
-	new_timer->it.cpu.task = p;
+	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);
 	return 0;
 }
 static int posix_cpu_timer_del(struct k_itimer *timer)
 {
 	struct cpu_timer *ctmr = &timer->it.cpu;
-	struct task_struct *p = ctmr->task;
 	struct sighand_struct *sighand;
+	struct task_struct *p;
 	unsigned long flags;
 	int ret = 0;
-	if (WARN_ON_ONCE(!p))
+	rcu_read_lock();
-		return -EINVAL;
+	p = cpu_timer_task_rcu(timer);
+	if (!p)
+		goto out;
 	/*
 	 * Protect against sighand release/switch in exit/exec and process/
 		unlock_task_sighand(p, &flags);
 	}
+out:
+	rcu_read_unlock();
 	if (!ret)
-		put_task_struct(p);
+		put_pid(ctmr->pid);
 	return ret;
 }
 	clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock);
 	u64 old_expires, new_expires, old_incr, val;
 	struct cpu_timer *ctmr = &timer->it.cpu;
-	struct task_struct *p = ctmr->task;
 	struct sighand_struct *sighand;
+	struct task_struct *p;
 	unsigned long flags;
 	int ret = 0;
-	if (WARN_ON_ONCE(!p))
+	rcu_read_lock();
-		return -EINVAL;
+	p = cpu_timer_task_rcu(timer);
+	if (!p) {
+		/*
+		 * If p has just been reaped, we can no
+		 * longer get any information about it at all.
+		 */
+		rcu_read_unlock();
+		return -ESRCH;
+	}
 	/*
 	 * Use the to_ktime conversion because that clamps the maximum
 	 * If p has just been reaped, we can no
 	 * longer get any information about it at all.
 	 */
-	if (unlikely(sighand == NULL))
+	if (unlikely(sighand == NULL)) {
+		rcu_read_unlock();
 		return -ESRCH;
+	}
 	/*
 	 * Disarm any old timer after extracting its expiry time.
 	ret = 0;
  out:
+	rcu_read_unlock();
 	if (old)
 		old->it_interval = ns_to_timespec64(old_incr);
 	clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock);
 	struct cpu_timer *ctmr = &timer->it.cpu;
 	u64 now, expires = cpu_timer_getexpires(ctmr);
-	struct task_struct *p = ctmr->task;
+	struct task_struct *p;
-	if (WARN_ON_ONCE(!p))
+	rcu_read_lock();
-		return;
+	p = cpu_timer_task_rcu(timer);
+	if (!p)
+		goto out;
 	/*
 	 * Easy part: convert the reload time.
 	itp->it_interval = ktime_to_timespec64(timer->it_interval);
 	if (!expires)
-		return;
+		goto out;
 	/*
 	 * Sample the clock to take the difference with the expiry time.
 		itp->it_value.tv_nsec = 1;
 		itp->it_value.tv_sec = 0;
 	}
+out:
+	rcu_read_unlock();
 }
 #define MAX_COLLECTED	20
 static void posix_cpu_timer_rearm(struct k_itimer *timer)
 {
 	clockid_t clkid = CPUCLOCK_WHICH(timer->it_clock);
-	struct cpu_timer *ctmr = &timer->it.cpu;
+	struct task_struct *p;
-	struct task_struct *p = ctmr->task;
 	struct sighand_struct *sighand;
 	unsigned long flags;
 	u64 now;
-	if (WARN_ON_ONCE(!p))
+	rcu_read_lock();
-		return;
+	p = cpu_timer_task_rcu(timer);
+	if (!p)
+		goto out;
 	/*
 	 * Fetch the current sample and update the timer's expiry time.
 	/* Protect timer list r/w in arm_timer() */
 	sighand = lock_task_sighand(p, &flags);
 	if (unlikely(sighand == NULL))
-		return;
+		goto out;
 	/*
 	 * Now re-arm for the new expiry time.
 	 */
 	arm_timer(timer, p);
 	unlock_task_sighand(p, &flags);
+out:
+	rcu_read_unlock();
 }
 /**