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

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

[Arnd Bergmann]

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>

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

[Nathan Chancellor]

On Thu, Sep 25, 2025 at 08:14:35AM +0200, Arnd Bergmann wrote:
> 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.

Agreed, I am not super familiar with this infrastructure but this seems
correct from my basic understanding.

> 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).

Yeah, that seems likely. For the record, I would not have felt
responsible for this code even if it was Cc'd to me since this does not
read as Kbuild material to me.

> 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>

I do not have a strong opinion on whether you or Andrew take it (I think
it makes sense either way) but I do not think it makes sense for me to
take it via Kbuild.

Cheers,
Nathan