Re: [PATCH v3] once: fix race by moving DO_ONCE to separate section

["Arnd Bergmann" <arnd@arndb.de>]

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@huawei.com>
> Signed-off-by: Qi Xi <xiqi2@huawei.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@arndb.de>