mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] srcutiny: Add reader flavor checking for atomic SRCU
@ 2026-09-13 13:43 Bradley Morgan
  2026-09-13 20:37 ` Paul E. McKenney
  0 siblings, 1 reply; 2+ messages in thread
From: Bradley Morgan @ 2026-09-13 13:43 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: jiangshanlai, josh, work, rostedt, mathieu.desnoyers, rcu,
	linux-kernel, brads

Tiny SRCU stubs out srcu_check_read_flavor() as a nop.  So if you
DEFINE_SRCU_ATOMIC() a struct and then call srcu_read_lock() on it
by mistake, nothing warns you.  Tree SRCU has
__srcu_check_read_flavor() which splats when the flavor changes.

Add a srcu_reader_flavor field to Tiny's srcu_struct, gated on
CONFIG_PROVE_RCU.  DEFINE_SRCU_ATOMIC() bakes in the flavor at
build time via __SRCU_STRUCT_INIT_ATOMIC(), and the runtime init
paths set it too.

The check itself is simple: warn if the flavor changes after first
use.  Tiny is UP so there is no race on first write, no cmpxchg
needed.

This catches the case where a normal reader sneaks into an atomic
domain.  synchronize_srcu_atomic() would then return early while
that reader is still in its section, which is a UAF waiting to
happen.

Signed-off-by: Bradley Morgan <brads@mainlining.org>
---
 include/linux/srcu.h     |  4 ++--
 include/linux/srcutiny.h | 39 +++++++++++++++++++++++++++++++++++----
 kernel/rcu/srcutiny.c    | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 1a8a465a5650..649a44d1cd00 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -36,9 +36,9 @@ static inline int __init_srcu_struct(struct srcu_struct *ssp, const char *name,
 int __init_srcu_struct_fast(struct srcu_struct *ssp, const char *name, struct lock_class_key *key);
 int __init_srcu_struct_fast_updown(struct srcu_struct *ssp, const char *name,
 				   struct lock_class_key *key);
+#endif // #ifndef CONFIG_TINY_SRCU
 int __init_srcu_struct_atomic(struct srcu_struct *ssp, const char *name,
 			      struct lock_class_key *key);
-#endif // #ifndef CONFIG_TINY_SRCU
 
 #define init_srcu_struct_fast(ssp) \
 ({ \
@@ -73,8 +73,8 @@ static inline int __init_srcu_struct(struct srcu_struct *ssp, const char *name,
 #ifndef CONFIG_TINY_SRCU
 int init_srcu_struct_fast(struct srcu_struct *ssp);
 int init_srcu_struct_fast_updown(struct srcu_struct *ssp);
-int init_srcu_struct_atomic(struct srcu_struct *ssp);
 #endif // #ifndef CONFIG_TINY_SRCU
+int init_srcu_struct_atomic(struct srcu_struct *ssp);
 
 #define __SRCU_DEP_MAP_INIT(srcu_name)
 #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
index 47a368f945e3..6b14511d09f2 100644
--- a/include/linux/srcutiny.h
+++ b/include/linux/srcutiny.h
@@ -20,6 +20,9 @@ struct srcu_struct {
 	u8 srcu_gp_running;		/* GP workqueue running? */
 	u8 srcu_gp_waiting;		/* GP waiting for readers? */
 	u8 srcu_atomic_gp_flag;		/* Serialize atomic GP work.*/
+#ifdef CONFIG_PROVE_RCU
+	u8 srcu_reader_flavor;		/* Reader flavor for srcu_struct? */
+#endif /* #ifdef CONFIG_PROVE_RCU */
 	unsigned long srcu_idx;		/* Current reader array element in bit 0x2. */
 	unsigned long srcu_idx_max;	/* Furthest future srcu_idx request. */
 	struct swait_queue_head srcu_wq;
@@ -39,6 +42,12 @@ void srcu_drive_gp(struct work_struct *wp);
 void srcu_tiny_irq_work(struct irq_work *irq_work);
 void srcu_defer_drain(struct irq_work *irq_work);
 
+#ifdef CONFIG_PROVE_RCU
+#define __SRCU_READER_FLAVOR_INIT	.srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC,
+#else
+#define __SRCU_READER_FLAVOR_INIT
+#endif
+
 #define __SRCU_STRUCT_INIT(name, __ignored, ___ignored, ____ignored)	\
 {									\
 	.srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq),	\
@@ -51,6 +60,19 @@ void srcu_defer_drain(struct irq_work *irq_work);
 	__SRCU_DEP_MAP_INIT(name)					\
 }
 
+#define __SRCU_STRUCT_INIT_ATOMIC(name)					\
+{									\
+	.srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq),	\
+	.srcu_cb_tail = &name.srcu_cb_head,				\
+	.srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp),	\
+	.srcu_irq_work = { .func = srcu_tiny_irq_work },		\
+	.defer_cbs = LLIST_HEAD_INIT(name.defer_cbs),			\
+	.defer_iw = { .node = { .u_flags = IRQ_WORK_HARD_IRQ },		\
+		      .func = srcu_defer_drain },			\
+	__SRCU_READER_FLAVOR_INIT					\
+	__SRCU_DEP_MAP_INIT(name)					\
+}
+
 /*
  * This odd _STATIC_ arrangement is needed for API compatibility with
  * Tree SRCU, which needs some per-CPU data.
@@ -65,20 +87,19 @@ void srcu_defer_drain(struct irq_work *irq_work);
 #define DEFINE_SRCU_FAST_UPDOWN(name) DEFINE_SRCU(name)
 #define DEFINE_STATIC_SRCU_FAST_UPDOWN(name) \
 	static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, name)
-#define DEFINE_SRCU_ATOMIC(name) DEFINE_SRCU(name)
+#define DEFINE_SRCU_ATOMIC(name) \
+	struct srcu_struct name = __SRCU_STRUCT_INIT_ATOMIC(name)
 #define DEFINE_STATIC_SRCU_ATOMIC(name) \
-	static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, name)
+	static struct srcu_struct name = __SRCU_STRUCT_INIT_ATOMIC(name)
 
 // Dummy structure for srcu_notifier_head.
 struct srcu_usage { };
 #define __SRCU_USAGE_INIT(name) { }
 #define __init_srcu_struct_fast __init_srcu_struct
 #define __init_srcu_struct_fast_updown __init_srcu_struct
-#define __init_srcu_struct_atomic __init_srcu_struct
 #ifndef CONFIG_DEBUG_LOCK_ALLOC
 #define init_srcu_struct_fast init_srcu_struct
 #define init_srcu_struct_fast_updown init_srcu_struct
-#define init_srcu_struct_atomic init_srcu_struct
 #endif // #ifndef CONFIG_DEBUG_LOCK_ALLOC
 
 void synchronize_srcu(struct srcu_struct *ssp);
@@ -147,7 +168,17 @@ static inline void synchronize_srcu_expedited(struct srcu_struct *ssp)
 void srcu_barrier(struct srcu_struct *ssp);
 
 static inline void srcu_expedite_current(struct srcu_struct *ssp) { }
+
+#ifdef CONFIG_PROVE_RCU
+void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
+
+static inline void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
+{
+	__srcu_check_read_flavor(ssp, read_flavor);
+}
+#else /* #ifdef CONFIG_PROVE_RCU */
 #define srcu_check_read_flavor(ssp, read_flavor) do { } while (0)
+#endif /* #else #ifdef CONFIG_PROVE_RCU */
 
 /* Defined here to avoid size increase for non-torture kernels. */
 static inline void srcu_torture_stats_print(struct srcu_struct *ssp,
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 76411562366f..76a276de07ee 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -52,6 +52,25 @@ static int init_srcu_struct_fields(struct srcu_struct *ssp)
 	return 0;
 }
 
+#ifdef CONFIG_PROVE_RCU
+int __init_srcu_struct_atomic(struct srcu_struct *ssp, const char *name,
+			      struct lock_class_key *key)
+{
+	ssp->srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC;
+	return __init_srcu_struct(ssp, name, key);
+}
+EXPORT_SYMBOL_GPL(__init_srcu_struct_atomic);
+
+#ifndef CONFIG_DEBUG_LOCK_ALLOC
+int init_srcu_struct_atomic(struct srcu_struct *ssp)
+{
+	ssp->srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC;
+	return init_srcu_struct_generic(ssp);
+}
+EXPORT_SYMBOL_GPL(init_srcu_struct_atomic);
+#endif /* #ifndef CONFIG_DEBUG_LOCK_ALLOC */
+#endif /* #ifdef CONFIG_PROVE_RCU */
+
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 
 int init_srcu_struct_lockdep(struct srcu_struct *ssp, const char *name,
@@ -212,6 +231,23 @@ void srcu_tiny_irq_work(struct irq_work *irq_work)
 }
 EXPORT_SYMBOL_GPL(srcu_tiny_irq_work);
 
+#ifdef CONFIG_PROVE_RCU
+/*
+ * Check for consistent reader flavor.  Tiny SRCU is UP only, so no
+ * cmpxchg is needed to set the flavor on first use.
+ */
+void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
+{
+	int old_read_flavor = ssp->srcu_reader_flavor;
+
+	WARN_ON_ONCE(read_flavor & (read_flavor - 1));
+	WARN_ON_ONCE(old_read_flavor && read_flavor != old_read_flavor);
+	if (!old_read_flavor)
+		ssp->srcu_reader_flavor = read_flavor;
+}
+EXPORT_SYMBOL_GPL(__srcu_check_read_flavor);
+#endif /* #ifdef CONFIG_PROVE_RCU */
+
 static void srcu_gp_start_if_needed(struct srcu_struct *ssp)
 {
 	unsigned long cookie;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] srcutiny: Add reader flavor checking for atomic SRCU
  2026-09-13 13:43 [PATCH] srcutiny: Add reader flavor checking for atomic SRCU Bradley Morgan
@ 2026-09-13 20:37 ` Paul E. McKenney
  0 siblings, 0 replies; 2+ messages in thread
From: Paul E. McKenney @ 2026-09-13 20:37 UTC (permalink / raw)
  To: Bradley Morgan
  Cc: jiangshanlai, josh, work, rostedt, mathieu.desnoyers, rcu, linux-kernel

On Sun, Sep 13, 2026 at 01:43:01PM +0000, Bradley Morgan wrote:
> Tiny SRCU stubs out srcu_check_read_flavor() as a nop.  So if you
> DEFINE_SRCU_ATOMIC() a struct and then call srcu_read_lock() on it
> by mistake, nothing warns you.  Tree SRCU has
> __srcu_check_read_flavor() which splats when the flavor changes.
> 
> Add a srcu_reader_flavor field to Tiny's srcu_struct, gated on
> CONFIG_PROVE_RCU.  DEFINE_SRCU_ATOMIC() bakes in the flavor at
> build time via __SRCU_STRUCT_INIT_ATOMIC(), and the runtime init
> paths set it too.
> 
> The check itself is simple: warn if the flavor changes after first
> use.  Tiny is UP so there is no race on first write, no cmpxchg
> needed.
> 
> This catches the case where a normal reader sneaks into an atomic
> domain.  synchronize_srcu_atomic() would then return early while
> that reader is still in its section, which is a UAF waiting to
> happen.
> 
> Signed-off-by: Bradley Morgan <brads@mainlining.org>

Great minds think alike?  Kunwu Chan recently proposed a similar change,
but after discussion, we chose not to take this approach on the grounds
that Tiny SRCU should remain tiny.

But maybe we missed something.  If so, please let us know what that is.

							Thanx, Paul

> ---
>  include/linux/srcu.h     |  4 ++--
>  include/linux/srcutiny.h | 39 +++++++++++++++++++++++++++++++++++----
>  kernel/rcu/srcutiny.c    | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index 1a8a465a5650..649a44d1cd00 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -36,9 +36,9 @@ static inline int __init_srcu_struct(struct srcu_struct *ssp, const char *name,
>  int __init_srcu_struct_fast(struct srcu_struct *ssp, const char *name, struct lock_class_key *key);
>  int __init_srcu_struct_fast_updown(struct srcu_struct *ssp, const char *name,
>  				   struct lock_class_key *key);
> +#endif // #ifndef CONFIG_TINY_SRCU
>  int __init_srcu_struct_atomic(struct srcu_struct *ssp, const char *name,
>  			      struct lock_class_key *key);
> -#endif // #ifndef CONFIG_TINY_SRCU
>  
>  #define init_srcu_struct_fast(ssp) \
>  ({ \
> @@ -73,8 +73,8 @@ static inline int __init_srcu_struct(struct srcu_struct *ssp, const char *name,
>  #ifndef CONFIG_TINY_SRCU
>  int init_srcu_struct_fast(struct srcu_struct *ssp);
>  int init_srcu_struct_fast_updown(struct srcu_struct *ssp);
> -int init_srcu_struct_atomic(struct srcu_struct *ssp);
>  #endif // #ifndef CONFIG_TINY_SRCU
> +int init_srcu_struct_atomic(struct srcu_struct *ssp);
>  
>  #define __SRCU_DEP_MAP_INIT(srcu_name)
>  #endif /* #else #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> index 47a368f945e3..6b14511d09f2 100644
> --- a/include/linux/srcutiny.h
> +++ b/include/linux/srcutiny.h
> @@ -20,6 +20,9 @@ struct srcu_struct {
>  	u8 srcu_gp_running;		/* GP workqueue running? */
>  	u8 srcu_gp_waiting;		/* GP waiting for readers? */
>  	u8 srcu_atomic_gp_flag;		/* Serialize atomic GP work.*/
> +#ifdef CONFIG_PROVE_RCU
> +	u8 srcu_reader_flavor;		/* Reader flavor for srcu_struct? */
> +#endif /* #ifdef CONFIG_PROVE_RCU */
>  	unsigned long srcu_idx;		/* Current reader array element in bit 0x2. */
>  	unsigned long srcu_idx_max;	/* Furthest future srcu_idx request. */
>  	struct swait_queue_head srcu_wq;
> @@ -39,6 +42,12 @@ void srcu_drive_gp(struct work_struct *wp);
>  void srcu_tiny_irq_work(struct irq_work *irq_work);
>  void srcu_defer_drain(struct irq_work *irq_work);
>  
> +#ifdef CONFIG_PROVE_RCU
> +#define __SRCU_READER_FLAVOR_INIT	.srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC,
> +#else
> +#define __SRCU_READER_FLAVOR_INIT
> +#endif
> +
>  #define __SRCU_STRUCT_INIT(name, __ignored, ___ignored, ____ignored)	\
>  {									\
>  	.srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq),	\
> @@ -51,6 +60,19 @@ void srcu_defer_drain(struct irq_work *irq_work);
>  	__SRCU_DEP_MAP_INIT(name)					\
>  }
>  
> +#define __SRCU_STRUCT_INIT_ATOMIC(name)					\
> +{									\
> +	.srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq),	\
> +	.srcu_cb_tail = &name.srcu_cb_head,				\
> +	.srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp),	\
> +	.srcu_irq_work = { .func = srcu_tiny_irq_work },		\
> +	.defer_cbs = LLIST_HEAD_INIT(name.defer_cbs),			\
> +	.defer_iw = { .node = { .u_flags = IRQ_WORK_HARD_IRQ },		\
> +		      .func = srcu_defer_drain },			\
> +	__SRCU_READER_FLAVOR_INIT					\
> +	__SRCU_DEP_MAP_INIT(name)					\
> +}
> +
>  /*
>   * This odd _STATIC_ arrangement is needed for API compatibility with
>   * Tree SRCU, which needs some per-CPU data.
> @@ -65,20 +87,19 @@ void srcu_defer_drain(struct irq_work *irq_work);
>  #define DEFINE_SRCU_FAST_UPDOWN(name) DEFINE_SRCU(name)
>  #define DEFINE_STATIC_SRCU_FAST_UPDOWN(name) \
>  	static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, name)
> -#define DEFINE_SRCU_ATOMIC(name) DEFINE_SRCU(name)
> +#define DEFINE_SRCU_ATOMIC(name) \
> +	struct srcu_struct name = __SRCU_STRUCT_INIT_ATOMIC(name)
>  #define DEFINE_STATIC_SRCU_ATOMIC(name) \
> -	static struct srcu_struct name = __SRCU_STRUCT_INIT(name, name, name, name)
> +	static struct srcu_struct name = __SRCU_STRUCT_INIT_ATOMIC(name)
>  
>  // Dummy structure for srcu_notifier_head.
>  struct srcu_usage { };
>  #define __SRCU_USAGE_INIT(name) { }
>  #define __init_srcu_struct_fast __init_srcu_struct
>  #define __init_srcu_struct_fast_updown __init_srcu_struct
> -#define __init_srcu_struct_atomic __init_srcu_struct
>  #ifndef CONFIG_DEBUG_LOCK_ALLOC
>  #define init_srcu_struct_fast init_srcu_struct
>  #define init_srcu_struct_fast_updown init_srcu_struct
> -#define init_srcu_struct_atomic init_srcu_struct
>  #endif // #ifndef CONFIG_DEBUG_LOCK_ALLOC
>  
>  void synchronize_srcu(struct srcu_struct *ssp);
> @@ -147,7 +168,17 @@ static inline void synchronize_srcu_expedited(struct srcu_struct *ssp)
>  void srcu_barrier(struct srcu_struct *ssp);
>  
>  static inline void srcu_expedite_current(struct srcu_struct *ssp) { }
> +
> +#ifdef CONFIG_PROVE_RCU
> +void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor);
> +
> +static inline void srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
> +{
> +	__srcu_check_read_flavor(ssp, read_flavor);
> +}
> +#else /* #ifdef CONFIG_PROVE_RCU */
>  #define srcu_check_read_flavor(ssp, read_flavor) do { } while (0)
> +#endif /* #else #ifdef CONFIG_PROVE_RCU */
>  
>  /* Defined here to avoid size increase for non-torture kernels. */
>  static inline void srcu_torture_stats_print(struct srcu_struct *ssp,
> diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> index 76411562366f..76a276de07ee 100644
> --- a/kernel/rcu/srcutiny.c
> +++ b/kernel/rcu/srcutiny.c
> @@ -52,6 +52,25 @@ static int init_srcu_struct_fields(struct srcu_struct *ssp)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PROVE_RCU
> +int __init_srcu_struct_atomic(struct srcu_struct *ssp, const char *name,
> +			      struct lock_class_key *key)
> +{
> +	ssp->srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC;
> +	return __init_srcu_struct(ssp, name, key);
> +}
> +EXPORT_SYMBOL_GPL(__init_srcu_struct_atomic);
> +
> +#ifndef CONFIG_DEBUG_LOCK_ALLOC
> +int init_srcu_struct_atomic(struct srcu_struct *ssp)
> +{
> +	ssp->srcu_reader_flavor = SRCU_READ_FLAVOR_ATOMIC;
> +	return init_srcu_struct_generic(ssp);
> +}
> +EXPORT_SYMBOL_GPL(init_srcu_struct_atomic);
> +#endif /* #ifndef CONFIG_DEBUG_LOCK_ALLOC */
> +#endif /* #ifdef CONFIG_PROVE_RCU */
> +
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
>  
>  int init_srcu_struct_lockdep(struct srcu_struct *ssp, const char *name,
> @@ -212,6 +231,23 @@ void srcu_tiny_irq_work(struct irq_work *irq_work)
>  }
>  EXPORT_SYMBOL_GPL(srcu_tiny_irq_work);
>  
> +#ifdef CONFIG_PROVE_RCU
> +/*
> + * Check for consistent reader flavor.  Tiny SRCU is UP only, so no
> + * cmpxchg is needed to set the flavor on first use.
> + */
> +void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
> +{
> +	int old_read_flavor = ssp->srcu_reader_flavor;
> +
> +	WARN_ON_ONCE(read_flavor & (read_flavor - 1));
> +	WARN_ON_ONCE(old_read_flavor && read_flavor != old_read_flavor);
> +	if (!old_read_flavor)
> +		ssp->srcu_reader_flavor = read_flavor;
> +}
> +EXPORT_SYMBOL_GPL(__srcu_check_read_flavor);
> +#endif /* #ifdef CONFIG_PROVE_RCU */
> +
>  static void srcu_gp_start_if_needed(struct srcu_struct *ssp)
>  {
>  	unsigned long cookie;
> -- 
> 2.47.3

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-13 23:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-13 13:43 [PATCH] srcutiny: Add reader flavor checking for atomic SRCU Bradley Morgan
2026-09-13 20:37 ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®