linux-kernel - [PATCH 3/3] sched, timer: Use cmpxchg to do updates in update_gt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1429052986-9420-4-git-send-email-jason.low2@hp.com>
Date:	Tue, 14 Apr 2015 16:09:46 -0700
From:	Jason Low <jason.low2@...com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>
Cc:	linux-kernel@...r.kernel.org,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Mike Galbraith <umgwanakikbuti@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Mel Gorman <mgorman@...e.de>,
	Steven Rostedt <rostedt@...dmis.org>,
	Preeti U Murthy <preeti@...ux.vnet.ibm.com>,
	hideaki.kimura@...com, Aswin Chandramouleeswaran <aswin@...com>,
	Scott J Norton <scott.norton@...com>,
	Jason Low <jason.low2@...com>
Subject: [PATCH 3/3] sched, timer: Use cmpxchg to do updates in update_gt_cputime()

Note: The chance that the race which this patch addresses seems very
unlikely to occur, especially after the change in patch 2 which sets
the running field after calling this update_gt_cputimer().

However, I am including this patch if we want to be completely safe
from concurrent updates.

-----------------------------------------------------------------------------

Since we're now updating thread group cputimer values without a lock,
there is now a potential race that can occur in update_gt_cputime() where
the cputimers are concurrently being updated in account_group_*_time().

This can occur when the ->running field transitions from 1 -> 0 -> 1. If the
cputimer->running field is set while thread 1 runs run_posix_cpu_timers(),
but another thread, thread 2, turns off cputimer->running before thread 1
enters thread_group_cputimer(), and another thread, thread 3, enables it
after thread 1 checks !cputimer->running in thread_group_cputimer(), then
there is a possibility that update_gt_cputime() is updating the cputimers
while the cputimer is running.

This patch uses cmpxchg and retry logic to ensure that update_gt_cputime()
is making its updates atomically.

Signed-off-by: Jason Low <jason.low2@...com>
---
 kernel/time/posix-cpu-timers.c |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 7e96082..130d717 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -196,16 +196,26 @@ static int cpu_clock_sample(const clockid_t which_clock, struct task_struct *p,
 	return 0;
 }
 
-static void update_gt_cputime(struct thread_group_cputimer *cputimer, struct task_cputime *sum)
+static inline void __update_gt_cputime(atomic64_t *cputime, u64 sum_cputime)
 {
-	if (sum->utime > atomic64_read(&cputimer->utime))
-		atomic64_set(&cputimer->utime, sum->utime);
-
-	if (sum->stime > atomic64_read(&cputimer->stime))
-		atomic64_set(&cputimer->stime, sum->stime);
+	u64 curr_cputime;
+	/*
+	 * Set cputime to sum_cputime if sum_cputime > cputime. Use cmpxchg
+	 * to avoid race conditions with concurrent updates to cputime.
+	 */
+retry:
+	curr_cputime = atomic64_read(cputime);
+	if (sum_cputime > curr_cputime) {
+		if (atomic64_cmpxchg(cputime, curr_cputime, sum_cputime) != curr_cputime)
+			goto retry;
+	}
+}
 
-	if (sum->sum_exec_runtime > atomic64_read(&cputimer->sum_exec_runtime))
-		atomic64_set(&cputimer->sum_exec_runtime, sum->sum_exec_runtime);
+static void update_gt_cputime(struct thread_group_cputimer *cputimer, struct task_cputime *sum)
+{
+	__update_gt_cputime(&cputimer->utime, sum->utime);
+	__update_gt_cputime(&cputimer->stime, sum->stime);
+	__update_gt_cputime(&cputimer->sum_exec_runtime, sum->sum_exec_runtime);
 }
 
 /* Sample thread_group_cputimer values in "cputimer", copy results to "times" */
-- 
1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/