On Thu, Oct 06, 2022 at 06:26:04AM -0600, Jason A. Donenfeld wrote:
> On Thu, Oct 06, 2022 at 08:46:27AM +0200, Sebastian Andrzej Siewior wrote:
> > On 2022-10-05 23:08:19 [+0200], Jason A. Donenfeld wrote:
> > > Hi Sebastian,
> > Hi Jason,
> >
> > > On Wed, Oct 05, 2022 at 07:26:42PM +0200, Sebastian Andrzej Siewior wrote:
> > > > That del_timer_sync() at the end is what you want. If the timer is
> > > > pending (as in enqueued in the timer wheel) then it will be removed
> > > > before it is invoked. If the timer's callback is invoked then it will
> > > > spin until the callback is done.
> > >
> > > del_timer_sync() is not guaranteed to succeed with add_timer_on() being
> > > used in conjunction with timer_pending() though. That's why I've
> > > abandoned this.
> >
> > But why? The timer is added to a timer-base on a different CPU. Should
> > work.
>
> So it's easier to talk about, I'll number a few lines:
>
> 1 while (conditions) {
> 2 if (!timer_pending(&stack.timer))
> 3 add_timer_on(&stack.timer, some_next_cpu);
> 4 }
> 5 del_timer_sync(&stack.timer);
>
>
> Then, steps to cause UaF:
>
> a) add_timer_on() on line 3 is called from CPU 1 and pends the timer on
> CPU 2.
>
> b) Just before the timer callback runs, not after, timer_pending() is
> made false, so the condition on line 2 holds true again.
>
> c) add_timer_on() on line 3 is called from CPU 1 and pends the timer on
> CPU 3.
>
> d) The conditions on line 1 are made false, and the loop breaks.
>
> e) del_timer_sync() on line 5 is called, and its `base->running_timer !=
> timer` check is false, because of step (c).
>
> f) `stack.timer` gets freed / goes out of scope.
>
> g) The callback scheduled from step (b) runs, and we have a UaF.
Here's a reproducer of this flow, which prints out:
[ 4.157610] wireguard: Stack on cpu 1 is corrupt
diff --git a/drivers/net/wireguard/main.c b/drivers/net/wireguard/main.c
index ee4da9ab8013..5c61f49918f2 100644
--- a/drivers/net/wireguard/main.c
+++ b/drivers/net/wireguard/main.c
@@ -17,10 +17,40 @@
#include <linux/genetlink.h>
#include <net/rtnetlink.h>
+struct state {
+ struct timer_list timer;
+ char valid[8];
+};
+
+static void fire(struct timer_list *timer)
+{
+ struct state *stack = container_of(timer, struct state, timer);
+ mdelay(1000);
+ pr_err("Stack on cpu %d is %s\n", raw_smp_processor_id(), stack->valid);
+}
+
+static void do_the_thing(struct work_struct *work)
+{
+ struct state stack = { .valid = "valid" };
+ timer_setup_on_stack(&stack.timer, fire, 0);
+ stack.timer.expires = jiffies;
+ add_timer_on(&stack.timer, 1);
+ while (timer_pending(&stack.timer))
+ cpu_relax();
+ stack.timer.expires = jiffies;
+ add_timer_on(&stack.timer, 2);
+ del_timer_sync(&stack.timer);
+ memcpy(&stack.valid, "corrupt", 8);
+}
+
+static DECLARE_DELAYED_WORK(reproducer, do_the_thing);
+
static int __init wg_mod_init(void)
{
int ret;
+ schedule_delayed_work_on(0, &reproducer, HZ * 3);
+
ret = wg_allowedips_slab_init();
if (ret < 0)
goto err_allowedips;
