linux-kernel - Re: [PATCH v6 4/6] timers: Add timer_shutdown_sync() to be called before freeing timers

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]

Date:   Mon, 14 Nov 2022 20:13:28 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Anna-Maria Gleixner <anna-maria@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Julia Lawall <Julia.Lawall@...ia.fr>
Subject: Re: [PATCH v6 4/6] timers: Add timer_shutdown_sync() to be called
 before freeing timers

On Mon, Nov 14 2022 at 08:36, Steven Rostedt wrote:
> On Mon, 14 Nov 2022 01:33:25 +0100
> Thomas Gleixner <tglx@...utronix.de> wrote:
>>    https://lore.kernel.org/all/87v8vjiaih.ffs@tglx/
>> 
> I'm not sure what you mean by that. The idea is that once timer_shutdown()
> is called, we still warn on re-arming the timer.

That's the whole point. As Linus and I discussed in that thread:

   "That would mean, that we still check the function pointer for NULL
    without warning and just return. That would indeed be a good argument
    for not having the warning at all."

and as I demonstrated you on the example of the BT driver which you
"fixed" this is the only sensible way to handle this.

The warning does not buy us anything, unless you want to go and amend
all the usage sites which trigger it with 'if (mystruct->shutdown)'
conditionals.

It's very similar to the work->canceling logic for kthreads that Linus
mentioned in this thread which prevents that the work timer is rearmed
concurrently. The difference is that timer_shutdown() is a final
decision which renders the timer unusable unless it is explicitely
reinitialized.

But that's mostly a matter of documentation and it has to be made clear
that nothing in a shutdown path which has the BT pattern:

     timer_shutdown();
     destroy_workqueue();

relies on the timer being functional after the shutdown point. I'm
pretty sure that the vast majority of such use cases do not care, but
given the size of the driver zoo I'm also sure that you'll find at least
one which depends on the timer working accross teardown.

Thanks,

        tglx