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

[Qi Xi]

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@xxxxxxxxxx> 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.