linux-kernel - Re: [PATCH] Revert "timers: Don't wake ktimersoftd on every tick"

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: <4b4df775-53b8-cdeb-381b-af8cabb364a8@ni.com>
Date:   Fri, 26 May 2017 15:25:50 -0500
From:   Haris Okanovic <haris.okanovic@...com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Anna-Maria Gleixner <anna-maria@...utronix.de>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        julia.cartwright@...com, gratian.crisan@...com
Subject: Re: [PATCH] Revert "timers: Don't wake ktimersoftd on every tick"

Oh crap. I think I see the problem. I decrement expired_count before 
processing the list. Dropping the lock permits another run of 
tick_find_expired()->find_expired_timers() in the middle of 
__expire_timers() since it uses expired_count==0 as a condition.

This should fix it, but I'll wait for Anna-Maria's test next week before 
submitting a patch.

>  static void expire_timers(struct timer_base *base)
>  {
>         struct hlist_head *head;
> +       int expCount = base->expired_count;
>
> -       while (base->expired_count--) {
> -               head = base->expired_lists + base->expired_count;
> +       while (expCount--) {
> +               head = base->expired_lists + expCount;
>                 __expire_timers(base, head);
>         }
>         base->expired_count = 0;
>  }

Thanks,
Haris


On 05/26/2017 02:49 PM, Thomas Gleixner wrote:
> On Fri, 26 May 2017, Haris Okanovic wrote:
>
>> Anna-Maria,
>>
>> Look-ahead is implemented by tick_find_expired() and expiry by __run_timers(),
>> both of which hold timer_base::lock (raw spin lock) while running. Those two
>> routines shouldn't be able to run simultaneously on the same timer_base. Are
>> you sure the race isn't in another code path?
>
> It happens when softirq runs and drops the spinlock to call the timer
> function. And from there stuff goes down the drain.
>
> Anna-Maria will send you the test case on monday.
>
> Thanks,
>
> 	tglx
>
> 	
>