itimer: resolve timer disarm deadlocks

Lockdep reports:

<4>[  180.883141] git/435 is trying to acquire lock:
<5>[  180.883538] 0xffffd0010366c010 (&s->signal_lock#2){+.-.}-{2:2}, at: kernel_raise_signal+0x6f/0x19f
<4>[  180.883538] but task is already holding lock:
<5>[  180.883982] 0xffff800507931e40 (&ev#2){+.-.}-{0:0}, at: _Z19timer_handle_eventsP5timer+0x160/0x2fa
<4>[  180.883982]
<5>[  180.900180]  Possible unsafe locking scenario:

<5>[  180.900180]        CPU0                    CPU1
<5>[  180.900180]        ----                    ----
<5>[  180.900180]   lock(&ev#2);
<5>[  180.900180]                                lock(&it.lock);
<5>[  180.900181]                                lock(&ev#2);
<5>[  180.900181]   lock(&s->signal_lock#2);
<5>[  180.900181]
 *** DEADLOCK ***

This happens because itimers firing require the signal lock (to send a
signal). Having the signal_lock (or something in its chain of
dependencies) while trying to cancel the timer results in a deadlock,
and a loud complaint from lockdep.

Fix it by finally adding a proper timer_cancel_try() which fails if the
timer is running. itimer_disarm() requires a back-out loop now, which in
exit's case needs to unlock+lock signal_lock.

Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>

Author: heatd

kernel/include/onyx/itimer.h

@@ -36,7 +36,7 @@ struct process;
 __BEGIN_CDECLS
 
 void itimer_init(struct process *p);
-void itimer_disarm(struct itimer *it);
+int itimer_disarm(struct itimer *it);
 
 __END_CDECLS
 #endif

kernel/include/onyx/timer.h

@@ -30,6 +30,15 @@ struct timer;
 struct clockevent;
 
 void timer_cancel_event(struct clockevent *ev);
+
+/**
+ * @brief Try to cancel a clockevent
+ *
+ * @param ev Event to cancel
+ * @retval true if still cancelled
+ * @return false if running
+ */
+bool timer_cancel_try(struct clockevent *ev);
 void timer_mod(struct clockevent *ev, hrtime_t future);
 struct clockevent
 {

kernel/kernel/exit.c

@@ -814,7 +814,21 @@ static void exit_signal(struct process *task)
         }
 
         for (int i = 0; i < ITIMER_COUNT; i++)
-            itimer_disarm(&sig->timers[i]);
+        {
+            int err;
+        again:
+            err = itimer_disarm(&sig->timers[i]);
+            if (err)
+            {
+                /* itimer_disarm found the timer running. This is problematic: the timer can try to
+                 * send a signal to ourselves, which grabs the signal_lock. To avoid this,
+                 * spin_unlock+relock and retry disarming. */
+                WARN_ON(err != -EAGAIN);
+                spin_unlock(&sighand->signal_lock);
+                spin_lock(&sighand->signal_lock);
+                goto again;
+            }
+        }
     }
 
     /* Remove ourselves from every list we've been apart of. Sibblings, tasklist, pids, threads */

kernel/kernel/timer.cpp

@@ -232,6 +232,18 @@ static void timer_spin_pending(struct timer *timer, struct clockevent *ev)
         cpu_relax();
 }
 
+/**
+ * @brief Try to cancel a clockevent
+ *
+ * @param ev Event to cancel
+ * @retval true if still cancelled
+ * @return false if running
+ */
+bool timer_cancel_try(struct clockevent *ev)
+{
+    return timer_cancel_event_try(ev) == nullptr;
+}
+
 void timer_cancel_event(struct clockevent *ev)
 {
     struct timer *timer;
@@ -419,13 +431,21 @@ int itimer::disarm()
     scoped_lock g{lock};
 
     if (armed)
-        timer_cancel_event(&ev);
+    {
+        if (!timer_cancel_try(&ev))
+        {
+            /* We can't form a dependency loop between signal_lock, itimer::lock and timer
+             * cancelling (waiting). Thus, back out and try again later. */
+            return -EAGAIN;
+        }
+    }
+
     return 0;
 }
 
-void itimer_disarm(struct itimer *it)
+int itimer_disarm(struct itimer *it)
 {
-    it->disarm();
+    return it->disarm();
 }
 
 int sys_setitimer(int which, const struct itimerval *new_value, struct itimerval *old_value)
@@ -460,7 +480,10 @@ int sys_setitimer(int which, const struct itimerval *new_value, struct itimerval
     auto &timer = current->sig->timers[which];
 
     if (!initial_ns)
-        st = timer.disarm();
+    {
+        while ((st = timer.disarm()) == -EAGAIN)
+            cpu_relax();
+    }
     else
         st = timer.arm(interval_ns, initial_ns);