linux-kernel - Re: [PATCH v2 rcu 0/8] NMI-safe SRCU reader API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Oct 2022 00:05:37 +0200
From:   Frederic Weisbecker <frederic@...nel.org>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     John Ogness <john.ogness@...utronix.de>, rcu@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel-team@...com,
        rostedt@...dmis.org, tglx@...utronix.de, pmladek@...e.com
Subject: Re: [PATCH v2 rcu 0/8] NMI-safe SRCU reader API

On Wed, Oct 19, 2022 at 12:14:18PM -0700, Paul E. McKenney wrote:
> On Wed, Oct 19, 2022 at 01:19:53PM +0206, John Ogness wrote:
> > On 2022-10-18, "Paul E. McKenney" <paulmck@...nel.org> wrote:
> > > And the v6.1-rc1 stack is now at srcunmisafe.2022.10.18b.
> > 
> > Thanks!
> > 
> > I guess the kernel test robot will catch this, but if you checkout
> > commit 79c95dc428ad ("arch/x86: Add ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
> > Kconfig option") and build for x86_64, you will get:
> > 
> > x86_64-linux-gnu-ld: kernel/rcu/srcutree.o: in function `srcu_gp_start_if_needed':
> > srcutree.c:(.text+0x133a): undefined reference to `__srcu_read_lock_nmisafe'
> > x86_64-linux-gnu-ld: srcutree.c:(.text+0x1490): undefined reference to `__srcu_read_unlock_nmisafe'
> > x86_64-linux-gnu-ld: kernel/rcu/srcutree.o: in function `srcu_barrier':
> > srcutree.c:(.text+0x1b03): undefined reference to `__srcu_read_lock_nmisafe'
> > x86_64-linux-gnu-ld: srcutree.c:(.text+0x1b38): undefined reference to `__srcu_read_unlock_nmisafe'
> > 
> > Note that this error is fixed with a later commit:
> > 
> > commit c2d158a284ab ("srcu: Debug NMI safety even on archs that don't
> > require it").
> > 
> > This does not affect what I am working on, so feel free to take care of
> > it whenever it fits your schedule.
> 
> Good catch, thank you!
> 
> It looks like the first two hunks in include/linux/srcu.h from that
> later commit need to be in that earlier commit.
> 
> Frederic, does this make sense, or am I off in the weeds?

Actually you need to do that earlier, in
6584822b1be1 ("srcu: Create an srcu_read_lock_nmisafe() and srcu_read_unlock_nmisafe()")

This way you don't only fix x86 bisectability but also the one of all the other safe archs.

And it's not just the first two hunks, you also need to include
the removal of the srcutiny.h/srcutree.h definitions.

So namely you need to apply the following to 6584822b1be1. You might
meet some minor retro-conflicts (the chknmisafe parameter didn't exist yet),
but that's pretty much it:

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 565f60d57484..f0814ffca34b 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -52,8 +52,6 @@ int init_srcu_struct(struct srcu_struct *ssp);
 #else
 /* Dummy definition for things like notifiers.  Actual use gets link error. */
 struct srcu_struct { };
-int __srcu_read_lock_nmisafe(struct srcu_struct *ssp, bool chknmisafe) __acquires(ssp);
-void __srcu_read_unlock_nmisafe(struct srcu_struct *ssp, int idx, bool chknmisafe) __releases(ssp);
 #endif
 
 void call_srcu(struct srcu_struct *ssp, struct rcu_head *head,
@@ -66,6 +64,20 @@ unsigned long get_state_synchronize_srcu(struct srcu_struct *ssp);
 unsigned long start_poll_synchronize_srcu(struct srcu_struct *ssp);
 bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie);
 
+#ifdef CONFIG_NEED_SRCU_NMI_SAFE
+int __srcu_read_lock_nmisafe(struct srcu_struct *ssp) __acquires(ssp);
+void __srcu_read_unlock_nmisafe(struct srcu_struct *ssp, int idx) __releases(ssp);
+#else
+static inline int __srcu_read_lock_nmisafe(struct srcu_struct *ssp)
+{
+	return __srcu_read_lock(ssp);
+}
+static inline void __srcu_read_unlock_nmisafe(struct srcu_struct *ssp, int idx)
+{
+	__srcu_read_unlock(ssp, idx);
+}
+#endif /* CONFIG_NEED_SRCU_NMI_SAFE */
+
 #ifdef CONFIG_SRCU
 void srcu_init(void);
 #else /* #ifdef CONFIG_SRCU */
diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
index f890301f123d..f3a4d65b91ef 100644
--- a/include/linux/srcutiny.h
+++ b/include/linux/srcutiny.h
@@ -89,16 +89,4 @@ static inline void srcu_torture_stats_print(struct srcu_struct *ssp,
 		 data_race(READ_ONCE(ssp->srcu_idx)),
 		 data_race(READ_ONCE(ssp->srcu_idx_max)));
 }
-
-static inline int __srcu_read_lock_nmisafe(struct srcu_struct *ssp, bool chknmisafe)
-{
-	BUG();
-	return 0;
-}
-
-static inline void __srcu_read_unlock_nmisafe(struct srcu_struct *ssp, int idx, bool chknmisafe)
-{
-	BUG();
-}
-
 #endif
diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index 35ffdedf86cc..c689a81752c9 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -159,7 +155,4 @@ void synchronize_srcu_expedited(struct srcu_struct *ssp);
 void srcu_barrier(struct srcu_struct *ssp);
 void srcu_torture_stats_print(struct srcu_struct *ssp, char *tt, char *tf);
 
-int __srcu_read_lock_nmisafe(struct srcu_struct *ssp, bool chknmisafe) __acquires(ssp);
-void __srcu_read_unlock_nmisafe(struct srcu_struct *ssp, int idx, bool chknmisafe) __releases(ssp);
-
 #endif