Use an IRQ alarm in rendez_sleep_timeout()
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 14 Jul 2016 14:04:18 +0000 (10:04 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 19 Jul 2016 15:43:10 +0000 (11:43 -0400)
Using an RKM alarm in this way is buggy.  Consider the case where the alarm
immediately fires, and we don't sleep.  "Fires", currently means the alarm
IRQ happened, not that the function ran!  Yikes.  So the RKM gets sent to
our core to run in the future.  Then we don't block (the rendez condition
happened or saw we weren't on the tchain), and we return.  Since we return,
we unwind the stack and basically free the alarm structure.  Later, the RKM
accesses this freed memory.  Based on some code comments, I was aware of
this possibility, but missed it when making rendez_sleep_timeout().

The real problem is unset_alarm() doesn't actually guarantee that the
memory is unused.  It should, which is the subject of a future patch.
However, it will basically require unset_alarm for RKM alarms to
potentially block, which we'd like to avoid in rendez code.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/src/rendez.c

index c2b3e87..80c29fa 100644 (file)
@@ -52,9 +52,11 @@ void rendez_sleep(struct rendez *rv, int (*cond)(void*), void *arg)
 /* Force a wakeup of all waiters on the rv, including non-timeout users.  For
  * those, they will just wake up, see the condition is still false (probably)
  * and go back to sleep. */
 /* Force a wakeup of all waiters on the rv, including non-timeout users.  For
  * those, they will just wake up, see the condition is still false (probably)
  * and go back to sleep. */
-static void rendez_alarm_handler(struct alarm_waiter *awaiter)
+static void rendez_alarm_handler(struct alarm_waiter *awaiter,
+                                 struct hw_trapframe *hw_tf)
 {
        struct rendez *rv = (struct rendez*)awaiter->data;
 {
        struct rendez *rv = (struct rendez*)awaiter->data;
+
        rendez_wakeup(rv);
 }
 
        rendez_wakeup(rv);
 }
 
@@ -78,8 +80,12 @@ void rendez_sleep_timeout(struct rendez *rv, int (*cond)(void*), void *arg,
        }
        cv_unlock_irqsave(&rv->cv, &irq_state);
        /* The handler will call rendez_wake, but won't mess with the condition
        }
        cv_unlock_irqsave(&rv->cv, &irq_state);
        /* The handler will call rendez_wake, but won't mess with the condition
-        * state.  It's enough to break us out of cv_wait() to see .on_tchain. */
-       init_awaiter(&awaiter, rendez_alarm_handler);
+        * state.  It's enough to break us out of cv_wait() to see .on_tchain.  But
+        * for this to work, we need to be an IRQ alarm handler.  If an RKM fired,
+        * then we'd be off the tchain, but it didn't actually run yet.  Also, since
+        * all we're doing is poking a rendez, we might as well just do it from IRQ
+        * ctx instead of mucking with an extra RKM. */
+       init_awaiter_irq(&awaiter, rendez_alarm_handler);
        awaiter.data = rv;
        set_awaiter_rel(&awaiter, usec);
        /* Set our alarm on this cpu's tchain.  Note that when we sleep in cv_wait,
        awaiter.data = rv;
        set_awaiter_rel(&awaiter, usec);
        /* Set our alarm on this cpu's tchain.  Note that when we sleep in cv_wait,