[RFC][PATCH] timer: Add function-change canary

[Kees Cook]

This introduces canaries to struct timer_list in an effort to protect the
function callback pointer from getting rewritten during stack or heap
overflow attacks. The struct timer_list has become a recent target for
security flaw exploitation because it includes the "data" argument in
the structure, along with the function callback. This provides attackers
with a ROP-like primitive for performing limited kernel function calls
without needing all the prerequisites to stage a ROP attack.

Recent examples of exploits using struct timer_list attacks:

http://www.openwall.com/lists/oss-security/2016/12/06/1
(https://www.exploit-db.com/exploits/40871/)

https://googleprojectzero.blogspot.com/2017/05/exploiting-linux-kernel-via-packet.html
(https://www.exploit-db.com/exploits/41458/)

Timers normally have their callback functions initialized either via
the setup_timer_*() macros or manually before calls to add_timer(). The
per-timer canary gets set in either case, and then checked at timer
expiration time before calling the function.

Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
 include/linux/timer.h |  6 ++++--
 kernel/time/timer.c   | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/include/linux/timer.h b/include/linux/timer.h
index e6789b8757d5..9aac0da9d2ec 100644
--- a/include/linux/timer.h
+++ b/include/linux/timer.h
@@ -16,6 +16,7 @@ struct timer_list {
 	 */
 	struct hlist_node	entry;
 	unsigned long		expires;
+	unsigned long		canary;
 	void			(*function)(unsigned long);
 	unsigned long		data;
 	u32			flags;
@@ -91,6 +92,7 @@ struct timer_list {
 
 void init_timer_key(struct timer_list *timer, unsigned int flags,
 		    const char *name, struct lock_class_key *key);
+void init_timer_func(struct timer_list *timer, void (*func)(unsigned long));
 
 #ifdef CONFIG_DEBUG_OBJECTS_TIMERS
 extern void init_timer_on_stack_key(struct timer_list *timer,
@@ -140,14 +142,14 @@ static inline void init_timer_on_stack_key(struct timer_list *timer,
 #define __setup_timer(_timer, _fn, _data, _flags)			\
 	do {								\
 		__init_timer((_timer), (_flags));			\
-		(_timer)->function = (_fn);				\
+		init_timer_func((_timer), (_fn));			\
 		(_timer)->data = (_data);				\
 	} while (0)
 
 #define __setup_timer_on_stack(_timer, _fn, _data, _flags)		\
 	do {								\
 		__init_timer_on_stack((_timer), (_flags));		\
-		(_timer)->function = (_fn);				\
+		init_timer_func((_timer), (_fn));			\
 		(_timer)->data = (_data);				\
 	} while (0)
 
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 71ce3f4eead3..bc8ae8ef9106 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -44,6 +44,7 @@
 #include <linux/sched/debug.h>
 #include <linux/slab.h>
 #include <linux/compat.h>
+#include <linux/random.h>
 
 #include <linux/uaccess.h>
 #include <asm/unistd.h>
@@ -1060,6 +1061,34 @@ int mod_timer(struct timer_list *timer, unsigned long expires)
 }
 EXPORT_SYMBOL(mod_timer);
 
+static DEFINE_MUTEX(timer_canary_mutex);
+static unsigned long timer_canary __ro_after_init;
+
+/**
+ * init_timer_func - set the function used for the timer
+ * @timer: the timer to be updated
+ * @func: the function to be called by the timer
+ *
+ * This should only be called once per timer creation to set the function.
+ * Normally used via the setup_timer_*() macros or add_timer().
+ */
+void init_timer_func(struct timer_list *timer, void (*func)(unsigned long))
+{
+	/* Initialize the global timer canary on first use. */
+	if (!timer_canary) {
+		mutex_lock(&timer_canary_mutex);
+		if (!timer_canary)
+			timer_canary = get_random_long();
+		mutex_unlock(&timer_canary_mutex);
+	}
+
+	/* Record timer canary for this timer function. */
+	timer->function = func;
+	timer->canary = (unsigned long)timer->function ^
+			(unsigned long)timer ^ timer_canary;
+}
+EXPORT_SYMBOL(init_timer_func);
+
 /**
  * add_timer - start a timer
  * @timer: the timer to be added
@@ -1077,6 +1106,7 @@ EXPORT_SYMBOL(mod_timer);
 void add_timer(struct timer_list *timer)
 {
 	BUG_ON(timer_pending(timer));
+	init_timer_func(timer, timer->function);
 	mod_timer(timer, timer->expires);
 }
 EXPORT_SYMBOL(add_timer);
@@ -1095,6 +1125,7 @@ void add_timer_on(struct timer_list *timer, int cpu)
 
 	BUG_ON(timer_pending(timer) || !timer->function);
 
+	init_timer_func(timer, timer->function);
 	new_base = get_timer_cpu_base(timer->flags, cpu);
 
 	/*
@@ -1244,6 +1275,7 @@ static void call_timer_fn(struct timer_list *timer, void (*fn)(unsigned long),
 			  unsigned long data)
 {
 	int count = preempt_count();
+	void (*badfn)(unsigned long) = NULL;
 
 #ifdef CONFIG_LOCKDEP
 	/*
@@ -1265,7 +1297,14 @@ static void call_timer_fn(struct timer_list *timer, void (*fn)(unsigned long),
 	lock_map_acquire(&lockdep_map);
 
 	trace_timer_expire_entry(timer);
-	fn(data);
+
+	/* Make sure the timer function hasn't changed since canary set. */
+	if (((unsigned long)fn ^ (unsigned long)timer ^ timer->canary) !=
+	    timer_canary) {
+		badfn = fn;
+	} else
+		fn(data);
+
 	trace_timer_expire_exit(timer);
 
 	lock_map_release(&lockdep_map);
@@ -1281,6 +1320,10 @@ static void call_timer_fn(struct timer_list *timer, void (*fn)(unsigned long),
 		 */
 		preempt_count_set(count);
 	}
+
+	WARN_RATELIMIT(badfn,
+		"timer: corrupt timer function (was %pF)\n",
+		(void *)((unsigned long)timer ^ timer->canary ^ timer_canary));
 }
 
 static void expire_timers(struct timer_base *base, struct hlist_head *head)
-- 
2.7.4


-- 
Kees Cook
Pixel Security