ucounts: Fix signal ucount refcounting

In commit fda31c50292a ("signal: avoid double atomic counter increments for user accounting") Linus made a clever optimization to how rlimits and the struct user_struct. Unfortunately that optimization does not work in the obvious way when moved to nested rlimits. The problem is that the last decrement of the per user namespace per user sigpending counter might also be the last decrement of the sigpending counter in the parent user namespace as well. Which means that simply freeing the leaf ucount in __free_sigqueue is not enough. Maintain the optimization and handle the tricky cases by introducing inc_rlimit_get_ucounts and dec_rlimit_put_ucounts. By moving the entire optimization into functions that perform all of the work it becomes possible to ensure that every level is handled properly. The new function inc_rlimit_get_ucounts returns 0 on failure to increment the ucount. This is different than inc_rlimit_ucounts which increments the ucounts and returns LONG_MAX if the ucount counter has exceeded it's maximum or it wrapped (to indicate the counter needs to decremented). I wish we had a single user to account all pending signals to across all of the threads of a process so this complexity was not necessary

Eric W. Biederman committed 5 years ago

Commit 15bc01e1 parent 7d2a07b3 files · +57 −19Browse files

Unified · Split

3 files

include/linux/user_namespace.h +2 −0
kernel/signal.c +6 −19
kernel/ucount.c +49 −0

include/linux/user_namespace.h+2−0

@@ -127,6 +127,8 @@ static inline long get_ucounts_value(struct ucounts *ucounts, enum ucount_type t · 126 unmodified lines
127		127
128	long inc_rlimit_ucounts(struct ucounts *ucounts, enum ucount_type type, long v);	128	long inc_rlimit_ucounts(struct ucounts *ucounts, enum ucount_type type, long v);
129	bool dec_rlimit_ucounts(struct ucounts *ucounts, enum ucount_type type, long v);	129	bool dec_rlimit_ucounts(struct ucounts *ucounts, enum ucount_type type, long v);
		130	long inc_rlimit_get_ucounts(struct ucounts *ucounts, enum ucount_type type);
		131	void dec_rlimit_put_ucounts(struct ucounts *ucounts, enum ucount_type type);
130	bool is_ucounts_overlimit(struct ucounts *ucounts, enum ucount_type type, unsigned long max);	132	bool is_ucounts_overlimit(struct ucounts *ucounts, enum ucount_type type, unsigned long max);
131		133
132	static inline void set_rlimit_ucount_max(struct user_namespace *ns,	134	static inline void set_rlimit_ucount_max(struct user_namespace *ns,

kernel/signal.c+6−19

 	 */
 	rcu_read_lock();
 	ucounts = task_ucounts(t);
-	sigpending = inc_rlimit_ucounts(ucounts, UCOUNT_RLIMIT_SIGPENDING, 1);
+	sigpending = inc_rlimit_get_ucounts(ucounts, UCOUNT_RLIMIT_SIGPENDING);
-	switch (sigpending) {
-	case 1:
-		if (likely(get_ucounts(ucounts)))
-			break;
-		fallthrough;
-	case LONG_MAX:
-		/*
-		 * we need to decrease the ucount in the userns tree on any
-		 * failure to avoid counts leaking.
-		 */
-		dec_rlimit_ucounts(ucounts, UCOUNT_RLIMIT_SIGPENDING, 1);
-		rcu_read_unlock();
-		return NULL;
-	}
 	rcu_read_unlock();
+	if (!sigpending)
+		return NULL;
 	if (override_rlimit || likely(sigpending <= task_rlimit(t, RLIMIT_SIGPENDING))) {
 		q = kmem_cache_alloc(sigqueue_cachep, gfp_flags);
 	}
 	if (unlikely(q == NULL)) {
-		if (dec_rlimit_ucounts(ucounts, UCOUNT_RLIMIT_SIGPENDING, 1))
+		dec_rlimit_put_ucounts(ucounts, UCOUNT_RLIMIT_SIGPENDING);
-			put_ucounts(ucounts);
 	} else {
 		INIT_LIST_HEAD(&q->list);
 		q->flags = sigqueue_flags;
 {
 	if (q->flags & SIGQUEUE_PREALLOC)
 		return;
-	if (q->ucounts && dec_rlimit_ucounts(q->ucounts, UCOUNT_RLIMIT_SIGPENDING, 1)) {
+	if (q->ucounts) {
-		put_ucounts(q->ucounts);
+		dec_rlimit_put_ucounts(q->ucounts, UCOUNT_RLIMIT_SIGPENDING);
 		q->ucounts = NULL;
 	}
 	kmem_cache_free(sigqueue_cachep, q);

kernel/ucount.c+49−0

@@ -284,6 +284,55 @@ bool dec_rlimit_ucounts(struct ucounts *ucounts, enum ucount_type type, long v) · 283 unmodified lines
284	return (new == 0);	284	return (new == 0);
285	}	285	}
286		286
		287	static void do_dec_rlimit_put_ucounts(struct ucounts *ucounts,
		288	struct ucounts *last, enum ucount_type type)
		289	{
		290	struct ucounts iter, next;
		291	for (iter = ucounts; iter != last; iter = next) {
		292	long dec = atomic_long_add_return(-1, &iter->ucount[type]);
		293	WARN_ON_ONCE(dec < 0);
		294	next = iter->ns->ucounts;
		295	if (dec == 0)
		296	put_ucounts(iter);
		297	}
		298	}
		299
		300	void dec_rlimit_put_ucounts(struct ucounts *ucounts, enum ucount_type type)
		301	{
		302	do_dec_rlimit_put_ucounts(ucounts, NULL, type);
		303	}
		304
		305	long inc_rlimit_get_ucounts(struct ucounts *ucounts, enum ucount_type type)
		306	{
		307	/* Caller must hold a reference to ucounts */
		308	struct ucounts *iter;
		309	long dec, ret = 0;
		310
		311	for (iter = ucounts; iter; iter = iter->ns->ucounts) {
		312	long max = READ_ONCE(iter->ns->ucount_max[type]);
		313	long new = atomic_long_add_return(1, &iter->ucount[type]);
		314	if (new < 0 \|\| new > max)
		315	goto unwind;
		316	if (iter == ucounts)
		317	ret = new;
		318	/*
		319	* Grab an extra ucount reference for the caller when
		320	* the rlimit count was previously 0.
		321	*/
		322	if (new != 1)
		323	continue;
		324	if (!get_ucounts(iter))
		325	goto dec_unwind;
		326	}
		327	return ret;
		328	dec_unwind:
		329	dec = atomic_long_add_return(-1, &iter->ucount[type]);
		330	WARN_ON_ONCE(dec < 0);
		331	unwind:
		332	do_dec_rlimit_put_ucounts(ucounts, iter, type);
		333	return 0;
		334	}
		335
287	bool is_ucounts_overlimit(struct ucounts *ucounts, enum ucount_type type, unsigned long max)	336	bool is_ucounts_overlimit(struct ucounts *ucounts, enum ucount_type type, unsigned long max)
288	{	337	{
289	struct ucounts *iter;	338	struct ucounts *iter;