Messages in this thread | | | Date | Tue, 31 May 2022 12:33:16 -0700 | From | Kees Cook <> | Subject | Re: [PATCH] cfi: Fix __cfi_slowpath_diag RCU usage with cpuidle |
| |
On Tue, May 31, 2022 at 10:59:10AM -0700, Sami Tolvanen wrote: > RCU_NONIDLE usage during __cfi_slowpath_diag can result in an invalid > RCU state in the cpuidle code path: > > WARNING: CPU: 1 PID: 0 at kernel/rcu/tree.c:613 rcu_eqs_enter+0xe4/0x138 > ... > Call trace: > rcu_eqs_enter+0xe4/0x138 > rcu_idle_enter+0xa8/0x100 > cpuidle_enter_state+0x154/0x3a8 > cpuidle_enter+0x3c/0x58 > do_idle.llvm.6590768638138871020+0x1f4/0x2ec > cpu_startup_entry+0x28/0x2c > secondary_start_kernel+0x1b8/0x220 > __secondary_switched+0x94/0x98 > > Instead, call rcu_irq_enter/exit to wake up RCU only when needed and > disable interrupts for the entire CFI shadow/module check when we do. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > kernel/cfi.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/kernel/cfi.c b/kernel/cfi.c > index 9594cfd1cf2c..08102d19ec15 100644 > --- a/kernel/cfi.c > +++ b/kernel/cfi.c > @@ -281,6 +281,8 @@ static inline cfi_check_fn find_module_check_fn(unsigned long ptr) > static inline cfi_check_fn find_check_fn(unsigned long ptr) > { > cfi_check_fn fn = NULL; > + unsigned long flags; > + bool rcu_idle; > > if (is_kernel_text(ptr)) > return __cfi_check; > @@ -290,13 +292,21 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr) > * the shadow and __module_address use RCU, so we need to wake it > * up if necessary. > */ > - RCU_NONIDLE({ > - if (IS_ENABLED(CONFIG_CFI_CLANG_SHADOW)) > - fn = find_shadow_check_fn(ptr); > + rcu_idle = !rcu_is_watching(); > + if (rcu_idle) { > + local_irq_save(flags); > + rcu_irq_enter(); > + } > + > + if (IS_ENABLED(CONFIG_CFI_CLANG_SHADOW)) > + fn = find_shadow_check_fn(ptr); > + if (!fn) > + fn = find_module_check_fn(ptr); > > - if (!fn) > - fn = find_module_check_fn(ptr); > - }); > + if (rcu_idle) { > + rcu_irq_exit(); > + local_irq_restore(flags); > + } > > return fn; > } > -- > 2.36.1.255.ge46751e96f-goog >
Thanks for the fix! It looks like these tags could be added, yes?
Fixes: cf68fffb66d6 ("add support for Clang CFI") Cc: stable@vger.kernel.org
Also, to improve the commit log, under what situation was this problem encountered?
-- Kees Cook
| |