Adapt the timer API Usage (#98)

Since v4.14 [1], the timer API has been changed to improve memory safety.
The series of improvements ended up at v4.15 [2].

Reference: https://lwn.net/Articles/735887/

Close #97

[1] https://github.com/torvalds/linux/commit/686fef928bba6be13cabe639f154af7d72b63120
[2] https://github.com/torvalds/linux/commit/841b86f3289dbe858daeceec36423d4ea286fac2

diff --git a/examples/kbleds.c b/examples/kbleds.c
index 07a2357f69ea31036f9b25a64b4f7eeb4721a904..9b7555a486b649fd503e6b4279c5108d330fe8c8 100644 (file)
--- a/examples/kbleds.c
+++ b/examples/kbleds.c
@@ -5,17 +5,16 @@
 #include <linux/init.h>
 #include <linux/kd.h> /* For KDSETLED */
 #include <linux/module.h>
-#include <linux/tty.h> /* For fg_console, MAX_NR_CONSOLES */
-#include <linux/vt.h>
+#include <linux/tty.h> /* For tty_struct */
+#include <linux/vt.h> /* For MAX_NR_CONSOLES */
 #include <linux/vt_kern.h> /* for fg_console */
-
 #include <linux/console_struct.h> /* For vc_cons */
 
 MODULE_DESCRIPTION("Example module illustrating the use of Keyboard LEDs.");
 
 static struct timer_list my_timer;
 static struct tty_driver *my_driver;
-static char kbledstatus = 0;
+static unsigned long kbledstatus = 0;
 
 #define BLINK_DELAY HZ / 5
 #define ALL_LEDS_ON 0x07
@@ -32,18 +31,16 @@ static char kbledstatus = 0;
  * the LEDs reflect the actual keyboard status).  To learn more on this,
  * please see file: drivers/tty/vt/keyboard.c, function setledstate().
  */
-
-static void my_timer_func(unsigned long ptr)
+static void my_timer_func(struct timer_list *unused)
 {
-    unsigned long *pstatus = (unsigned long *)ptr;
     struct tty_struct *t = vc_cons[fg_console].d->port.tty;
 
-    if (*pstatus == ALL_LEDS_ON)
-        *pstatus = RESTORE_LEDS;
+    if (kbledstatus == ALL_LEDS_ON)
+        kbledstatus = RESTORE_LEDS;
     else
-        *pstatus = ALL_LEDS_ON;
+        kbledstatus = ALL_LEDS_ON;
 
-    (my_driver->ops->ioctl)(t, KDSETLED, *pstatus);
+    (my_driver->ops->ioctl)(t, KDSETLED, kbledstatus);
 
     my_timer.expires = jiffies + BLINK_DELAY;
     add_timer(&my_timer);
@@ -67,7 +64,7 @@ static int __init kbleds_init(void)
     pr_info("kbleds: tty driver magic %x\n", my_driver->magic);
 
     /* Set up the LED blink timer the first time. */
-    timer_setup(&my_timer, (void *)&my_timer_func, (unsigned long)&kbledstatus);
+    timer_setup(&my_timer, my_timer_func, 0);
     my_timer.expires = jiffies + BLINK_DELAY;
     add_timer(&my_timer);
 

diff --git a/lkmpg.tex b/lkmpg.tex
index 5dbd0cab0089f39af88db04ac5839261febc8172..1fca5d333b31800420c56da4f3afa47ae995cdbf 100644 (file)
--- a/lkmpg.tex
+++ b/lkmpg.tex
@@ -1502,6 +1502,47 @@ In certain conditions, you may desire a simpler and more direct way to communica
 Flashing keyboard LEDs can be such a solution: It is an immediate way to attract attention or to display a status condition.
 Keyboard LEDs are present on every hardware, they are always visible, they do not need any setup, and their use is rather simple and non-intrusive, compared to writing to a tty or a file.
 
+From v4.14 to v4.15, the timer API made a series of changes to improve memory safety.
+A buffer overflow in the area of a \cpp|timer_list| structure may be able to overwrite the \cpp|function| and \cpp|data| fields, providing the attacker with a way to use return-object programming (ROP) to call arbitrary functions within the kernel.
+Also, the function prototype of the callback, containing a \cpp|unsigned long| argument, will prevent work from any type checking.
+Furthermore, the function prototype with \cpp|unsigned long| argument may be an obstacle to the \textit{control-flow integrity}.
+Thus, it is better to use a unique prototype to separate from the cluster that takes an \cpp|unsigned long| argument.
+The timer callback should be passed a pointer to the \cpp|timer_list| structure rather than an \cpp|unsigned long| argument.
+Then, it wraps all the information the callback needs, including the \cpp|timer_list| structure, into a larger structure, and it can use the \cpp|container_of| macro instead of the \cpp|unsigned long| value.
+
+Before Linux v4.14, \cpp|setup_timer| was used to initialize the timer and the \cpp|timer_list| structure looked like:
+\begin{code}
+struct timer_list {
+    unsigned long expires;
+    void (*function)(unsigned long);
+    unsigned long data;
+    u32 flags;
+    /* ... */
+};
+
+void setup_timer(struct timer_list *timer, void (*callback)(unsigned long),
+                 unsigned long data);
+\end{code}
+
+Since Linux v4.14, \cpp|timer_setup| is adopted and the kernel step by step converting to \cpp|timer_setup| from \cpp|setup_timer|.
+One of the reasons why API was changed is it need to coexist with the old version interface.
+Moreover, the \cpp|timer_setup| was implemented by \cpp|setup_timer| at first.
+\begin{code}
+void timer_setup(struct timer_list *timer,
+                 void (*callback)(struct timer_list *), unsigned int flags);
+\end{code}
+
+The \cpp|setup_timer| was then removed since v4.15.
+As a result, the \cpp|timer_list| structure had changed to the following.
+\begin{code}
+struct timer_list {
+    unsigned long expires;
+    void (*function)(struct timer_list *);
+    u32 flags;
+    /* ... */
+};
+\end{code}
+
 The following source code illustrates a minimal kernel module which, when loaded, starts blinking the keyboard LEDs until it is unloaded.
 
 \samplec{examples/kbleds.c}