lkml.org 
[lkml]   [2022]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net v2] net: rose: fix UAF bug caused by rose_t0timer_expiry
Hello,

On Wed, 6 Jul 2022 19:02:37 -0700 Jakub Kicinski:

> On Tue, 5 Jul 2022 20:56:10 +0800 Duoming Zhou wrote:
> > + del_timer_sync(&rose_neigh->t0timer);
>
> /**
> * del_timer_sync - deactivate a timer and wait for the handler to finish.
> [...]
> * Synchronization rules: Callers must prevent restarting of the timer,
> * otherwise this function is meaningless.
>
> how is the restarting prevented? If I'm looking right
> rose_t0timer_expiry() rearms the timer.

The del_timer_sync() could stop the timer that restart itself in
its timer callback function.

The root cause is shown below which is a part of code in
del_timer_sync:

do {
ret = try_to_del_timer_sync(timer);

if (unlikely(ret < 0)) {
del_timer_wait_running(timer);
cpu_relax();
}
} while (ret < 0);

https://elixir.bootlin.com/linux/latest/source/kernel/time/timer.c#L1381

If the timer callback function is running, the try_to_del_timer_sync
will return -1. Then, it will loop until the timer is not queued and
the handler is not running on any CPU.

Although the timer may restart itself in timer callback function, the
del_timer_sync could also stop it.

In order to further prove the del_timer_sync() could stop the timer that
restart itself in its timer handler, I wrote the following kernel module
whoes part of code is shown below:

=================================================================

struct timer_list my_timer;
static void my_timer_callback(struct timer_list *timer);
static void start_timer(void);

static void start_timer(void){
del_timer(&my_timer);
my_timer.expires = jiffies+HZ;
my_timer.function = my_timer_callback;
add_timer(&my_timer);
}

static void my_timer_callback(struct timer_list *timer){
printk("In my_timer_function");
printk("the jiffies is %ld\n",jiffies);
start_timer();
}

static int __init del_timer_sync_init(void)
{
int result;
printk("my_timer will be create.\n");
printk("the jiffies is :%ld\n", jiffies);
timer_setup(&my_timer,my_timer_callback,0);
result = mod_timer(&my_timer,jiffies + SIXP_TXDELAY);
printk("the mod_timer is :%d\n\n",result);
return 0;
}

static void __exit del_timer_sync_exit(void)
{
int result=del_timer_sync(&my_timer);
printk("the del_timer_sync is :%d\n\n", result);
}

=================================================================

The timer handler is running from interrupts and del_timer_sync() could stop
the timer that rewind itself in its timer handler, the result is shown below:

# insmod del_timer_sync.ko
[ 103.505857] my_timer will be create.
[ 103.505922] the jiffies is :4294770832
[ 103.506845] the mod_timer is :0
[ 103.506845]
# [ 103.532389] In my_timer_function
[ 103.532452] the jiffies is 4294770859
[ 104.576768] In my_timer_function
[ 104.577096] the jiffies is 4294771904
[ 105.600941] In my_timer_function
[ 105.601072] the jiffies is 4294772928
[ 106.625397] In my_timer_function
[ 106.625573] the jiffies is 4294773952
[ 107.648995] In my_timer_function
[ 107.649212] the jiffies is 4294774976
[ 108.673037] In my_timer_function
[ 108.673787] the jiffies is 4294776001
rmmod del_timer_sync.ko
[ 109.649482] the del_timer_sync is :1
[ 109.649482]
#

If we call another thread such as a work_queue or the code in other places
to restart the timer instead of in its timer handler, the del_timer_sync()
could not stop it.

Best regards,
Duoming Zhou
\
 
 \ /
  Last update: 2022-07-07 04:24    [W:0.112 / U:1.372 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site