There are UAF bugs caused by rose_t0timer_expiry(). The
root cause is that del_timer() could not stop the timer
handler that is running and there is no synchronization.
One of the race conditions is shown below:
(thread 1) | (thread 2)
| rose_device_event
| rose_rt_device_down
| rose_remove_neigh
rose_t0timer_expiry | rose_stop_t0timer(rose_neigh)
... | del_timer(&neigh->t0timer)
| kfree(rose_neigh) //[1]FREE
neigh->dce_mode //[2]USE |
The rose_neigh is deallocated in position [1] and use in
position [2].
The crash trace triggered by POC is like below:
BUG: KASAN: use-after-free in expire_timers+0x144/0x320
Write of size 8 at addr ffff888009b19658 by task swapper/0/0
...
Call Trace:
<IRQ>
dump_stack_lvl+0xbf/0xee
print_address_description+0x7b/0x440
print_report+0x101/0x230
? expire_timers+0x144/0x320
kasan_report+0xed/0x120
? expire_timers+0x144/0x320
expire_timers+0x144/0x320
__run_timers+0x3ff/0x4d0
run_timer_softirq+0x41/0x80
__do_softirq+0x233/0x544
...
This patch changes rose_stop_ftimer() and rose_stop_t0timer()
in rose_remove_neigh() to del_timer_sync() in order that the
timer handler could be finished before the resources such as
rose_neigh and so on are deallocated. As a result, the UAF
bugs could be mitigated.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Duoming Zhou <[email protected]>
---
Changes in v2:
- v2: Use del_timer_sync to stop timer in rose_remove_neigh.
net/rose/rose_route.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index fee6409c2bb..eb0b8197ac8 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -227,8 +227,8 @@ static void rose_remove_neigh(struct rose_neigh *rose_neigh)
{
struct rose_neigh *s;
- rose_stop_ftimer(rose_neigh);
- rose_stop_t0timer(rose_neigh);
+ del_timer_sync(&rose_neigh->ftimer);
+ del_timer_sync(&rose_neigh->t0timer);
skb_queue_purge(&rose_neigh->queue);
--
2.17.1
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.
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
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
On Tue, 5 Jul 2022 20:56:10 +0800 you wrote:
> There are UAF bugs caused by rose_t0timer_expiry(). The
> root cause is that del_timer() could not stop the timer
> handler that is running and there is no synchronization.
> One of the race conditions is shown below:
>
> (thread 1) | (thread 2)
> | rose_device_event
> | rose_rt_device_down
> | rose_remove_neigh
> rose_t0timer_expiry | rose_stop_t0timer(rose_neigh)
> ... | del_timer(&neigh->t0timer)
> | kfree(rose_neigh) //[1]FREE
> neigh->dce_mode //[2]USE |
>
> [...]
Here is the summary with links:
- [net,v2] net: rose: fix UAF bug caused by rose_t0timer_expiry
https://git.kernel.org/netdev/net/c/148ca0451807
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html