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

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

Posted by Qi Xi 10 hours ago

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

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 883dbac79da9..94850b52e5cc 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -384,6 +384,7 @@
 	__start_once = .;						\
 	*(.data..once)							\
 	__end_once = .;							\
+	*(.data..do_once)						\
 	STRUCT_ALIGN();							\
 	*(__tracepoints)						\
 	/* implement dynamic printk debug */				\
diff --git a/include/linux/once.h b/include/linux/once.h
index 30346fcdc799..449a0e34ad5a 100644
--- a/include/linux/once.h
+++ b/include/linux/once.h
@@ -46,7 +46,7 @@ void __do_once_sleepable_done(bool *done, struct static_key_true *once_key,
 #define DO_ONCE(func, ...)						     \
 	({								     \
 		bool ___ret = false;					     \
-		static bool __section(".data..once") ___done = false;	     \
+		static bool __section(".data..do_once") ___done = false;     \
 		static DEFINE_STATIC_KEY_TRUE(___once_key);		     \
 		if (static_branch_unlikely(&___once_key)) {		     \
 			unsigned long ___flags;				     \
@@ -64,7 +64,7 @@ void __do_once_sleepable_done(bool *done, struct static_key_true *once_key,
 #define DO_ONCE_SLEEPABLE(func, ...)						\
 	({									\
 		bool ___ret = false;						\
-		static bool __section(".data..once") ___done = false;		\
+		static bool __section(".data..do_once") ___done = false;	\
 		static DEFINE_STATIC_KEY_TRUE(___once_key);			\
 		if (static_branch_unlikely(&___once_key)) {			\
 			___ret = __do_once_sleepable_start(&___done);		\
-- 
2.33.0

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

Posted by Qi Xi 9 hours ago

On 09/09/2025 19: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)
When simulating concurrent execution between DO_ONCE() and
clear_warn_once (clears the entire .data..once section in __do_once_done()),
BUG_ON() can be easily reproduced.

+#include <asm/sections.h>
  void __do_once_done(bool *done, struct static_key_true *once_key,
                     unsigned long *flags, struct module *mod)
         __releases(once_lock)
  {
         *done = true;
         spin_unlock_irqrestore(&once_lock, *flags);
+       memset(__start_once, 0, __end_once - __start_once);
+       pr_info("__do_once_done done: %d\n", *done); // *done equals 0
         once_disable_jump(once_key, mod);
  }

>
> 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(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 883dbac79da9..94850b52e5cc 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -384,6 +384,7 @@
>   	__start_once = .;						\
>   	*(.data..once)							\
>   	__end_once = .;							\
> +	*(.data..do_once)						\
>   	STRUCT_ALIGN();							\
>   	*(__tracepoints)						\
>   	/* implement dynamic printk debug */				\
> diff --git a/include/linux/once.h b/include/linux/once.h
> index 30346fcdc799..449a0e34ad5a 100644
> --- a/include/linux/once.h
> +++ b/include/linux/once.h
> @@ -46,7 +46,7 @@ void __do_once_sleepable_done(bool *done, struct static_key_true *once_key,
>   #define DO_ONCE(func, ...)						     \
>   	({								     \
>   		bool ___ret = false;					     \
> -		static bool __section(".data..once") ___done = false;	     \
> +		static bool __section(".data..do_once") ___done = false;     \
>   		static DEFINE_STATIC_KEY_TRUE(___once_key);		     \
>   		if (static_branch_unlikely(&___once_key)) {		     \
>   			unsigned long ___flags;				     \
> @@ -64,7 +64,7 @@ void __do_once_sleepable_done(bool *done, struct static_key_true *once_key,
>   #define DO_ONCE_SLEEPABLE(func, ...)						\
>   	({									\
>   		bool ___ret = false;						\
> -		static bool __section(".data..once") ___done = false;		\
> +		static bool __section(".data..do_once") ___done = false;	\
>   		static DEFINE_STATIC_KEY_TRUE(___once_key);			\
>   		if (static_branch_unlikely(&___once_key)) {			\
>   			___ret = __do_once_sleepable_start(&___done);		\