linux-kernel - Re: [PATCH] timers: fix synchronization rules in comments of del_timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64ac5e50.24399.181bc9704ed.Coremail.duoming@zju.edu.cn>
Date:   Sat, 2 Jul 2022 09:47:02 +0800 (GMT+08:00)
From:   duoming@....edu.cn
To:     linux-kernel@...r.kernel.org
Cc:     jstultz@...gle.com, tglx@...utronix.de, sboyd@...nel.org,
        edumazet@...gle.com
Subject: Re: [PATCH] timers: fix synchronization rules in comments of
 del_timer_sync

Hello maintainers,

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] 
# 

The root cause is shown below:

	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 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. So, I think the comments should be changed to "Callers
must prevent restarting of the timer in other places except for the timer's
handler".

Best regards,
Duoming Zhou