linux-kernel - Re: [PATCH] once: fix race by moving DO

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 for Android: free password hash cracker in your pocket

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

Message-ID: <57ed90e4-7030-4088-85e2-a166736a86f0@huawei.com>
Date: Mon, 8 Sep 2025 09:51:02 +0800
From: Qi Xi <xiqi2@...wei.com>
To: Eric Dumazet <edumazet@...gle.com>
CC: <bobo.shaobowang@...wei.com>, <xiexiuqi@...wei.com>, <arnd@...db.de>,
	<masahiroy@...nel.org>, <kuba@...nel.org>, <linux-arch@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <xiqi2@...wei.com>
Subject: Re: [PATCH] once: fix race by moving DO_ONCE to separate section

Hello Eric,

DO_ONCE_LITE_IF() in once_lite.h is used by WARN_ON_ONCE() and other warning
macros. Keep its ___done flag in the .data..once section and allow resetting
by clear_warn_once, as originally intended.

In contrast, DO_ONCE() is used for functions like get_random_once() and
relies on its ___done flag for internal synchronization.  We should not 
reset
DO_ONCE() by clear_warn_once and move the flag to the new .data..do_once 
section.

I'll send the v2 patch with the comment.

Qi

On 06/09/2025 22:34, Eric Dumazet wrote:
> On Sat, Sep 6, 2025 at 6:58 AM Qi Xi <xiqi2@...wei.com> wrote:
>> The commit c2c60ea37e5b ("once: use __section(".data.once")") moved
>> DO_ONCE's ___done variable to .data.once section, which conflicts with
>> WARN_ONCE series macros that also use the same section.
>>
>> This creates a race condition when clear_warn_once is used:
>>
>> Thread 1 (DO_ONCE)             Thread 2 (DO_ONCE)
>> __do_once_start
>>      read ___done (false)
>>      acquire once_lock
>> execute func
>> __do_once_done
>>      write ___done (true)      __do_once_start
>>      release once_lock             // Thread 3 clear_warn_once reset ___done
>>                                    read ___done (false)
>>                                    acquire once_lock
>>                                execute func
>> schedule once_work            __do_once_done
>> once_deferred: OK                 write ___done (true)
>> static_branch_disable             release once_lock
>>                                schedule once_work
>>                                once_deferred:
>>                                    BUG_ON(!static_key_enabled)
> Should we  use this section as well in include/linux/once_lite.h ?
>
> Or add a comment there explaining that there is a difference
> between the two variants, I am not sure this was explicitly mentioned
> in the past.