linux-kernel - Re: [PATCH v3] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <3de848cc-0f01-4999-a4fc-1ff3cc11daa4@app.fastmail.com>
Date: Thu, 25 Sep 2025 08:14:35 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Qi Xi" <xiqi2@...wei.com>, bobo.shaobowang@...wei.com,
 xiexiuqi@...wei.com
Cc: linux-kernel@...r.kernel.org, "Masahiro Yamada" <masahiroy@...nel.org>,
 "Jakub Kicinski" <kuba@...nel.org>, "Eric Dumazet" <edumazet@...gle.com>,
 Linux-Arch <linux-arch@...r.kernel.org>, linux-kbuild@...r.kernel.org,
 "Nathan Chancellor" <nathan@...nel.org>,
 "Nicolas Schier" <nicolas@...sle.eu>,
 "Andrew Morton" <akpm@...ux-foundation.org>
Subject: Re: [PATCH v3] once: fix race by moving DO_ONCE to separate section

On Tue, Sep 9, 2025, at 13:29, Qi Xi wrote:
> The commit c2c60ea37e5b ("once: use __section(".data.once")") moved
> DO_ONCE's ___done variable to .data.once section, which conflicts with
> DO_ONCE_LITE() that also uses 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)
>
> DO_ONCE_LITE() 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.
>
> Fix it by isolating DO_ONCE's ___done into a separate .data..do_once section,
> shielding it from clear_warn_once.
>
> Fixes: c2c60ea37e5b ("once: use __section(".data.once")")
> Reported-by: Hulk Robot <hulkci@...wei.com>
> Signed-off-by: Qi Xi <xiqi2@...wei.com>
> ---
> v3 -> v2: apply the same section change to DO_ONCE_SLEEPABLE().
> v2 -> v1: add comments for DO_ONCE_LITE() and DO_ONCE().
> ---
>  include/asm-generic/vmlinux.lds.h | 1 +
>  include/linux/once.h              | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)

Hi,

I hadn't seen this until your ping and had a look since it
touches asm-generic. The patch looks correct to me, thanks for
addressing this and for the detailed patch description.

I think what happened is that nobody felt responsible for
applying it, between the networking (which originally
added the infrastructure) and kbuild maintainers (Masahiro
did the last changes to this bit, but recently handed
over maintenance to Nathan).

I've applied the fix to the asm-generic tree for 6.18 now
to be sure that it makes it in and gets linux-next testing,
but it's not my area either really.

It would be best to have another review though. If Nathan
or Andrew want to instead pick it up through one of their
trees, please add

Acked-by: Arnd Bergmann <arnd@...db.de>