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
| ||
|
Message-ID: <20221202002729.GA1524469@paulmck-ThinkPad-P17-Gen-1> Date: Thu, 1 Dec 2022 16:27:29 -0800 From: "Paul E. McKenney" <paulmck@...nel.org> To: Juergen Gross <jgross@...e.com> Cc: Josh Poimboeuf <jpoimboe@...nel.org>, Andrew Cooper <Andrew.Cooper3@...rix.com>, Peter Zijlstra <peterz@...radead.org>, "sfr@...b.auug.org.au" <sfr@...b.auug.org.au>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "sstabellini@...nel.org" <sstabellini@...nel.org>, "boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>, "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>, Thomas Gleixner <tglx@...utronix.de> Subject: Re: objtool warning for next-20221118 On Tue, Nov 29, 2022 at 11:56:55AM -0800, Paul E. McKenney wrote: > On Fri, Nov 25, 2022 at 06:30:35AM +0100, Juergen Gross wrote: > > On 24.11.22 17:39, Josh Poimboeuf wrote: > > > On Thu, Nov 24, 2022 at 08:47:47AM +0100, Juergen Gross wrote: > > > > > > +++ b/arch/x86/xen/smp_pv.c > > > > > > @@ -385,17 +385,9 @@ static void xen_pv_play_dead(void) /* used only > > > > > > with HOTPLUG_CPU */ > > > > > > { > > > > > > play_dead_common(); > > > > > > HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(smp_processor_id()), NULL); > > > > > > - cpu_bringup(); > > > > > > - /* > > > > > > - * commit 4b0c0f294 (tick: Cleanup NOHZ per cpu data on cpu down) > > > > > > - * clears certain data that the cpu_idle loop (which called us > > > > > > - * and that we return from) expects. The only way to get that > > > > > > - * data back is to call: > > > > > > - */ > > > > > > - tick_nohz_idle_enter(); > > > > > > - tick_nohz_idle_stop_tick_protected(); > > > > > > - cpuhp_online_idle(CPUHP_AP_ONLINE_IDLE); > > > > > > + /* FIXME: converge cpu_bringup_and_idle() and start_secondary() */ > > > > > > + cpu_bringup_and_idle(); > > > > > > > > > > I think this will leak stack memory. Multiple cpu offline/online cycles of > > > > > the same cpu will finally exhaust the idle stack. > > > > > > Doh! Of course... > > > > > > I was actually thinking ahead, to where eventually xen_pv_play_dead() > > > can call start_cpu0(), which can be changed to automatically reset the > > > stack pointer like this: > > > > > > SYM_CODE_START(start_cpu0) > > > ANNOTATE_NOENDBR > > > UNWIND_HINT_EMPTY > > > movq PER_CPU_VAR(pcpu_hot + X86_top_of_stack), %rax > > > leaq -PTREGS_SIZE(%rax), %rsp > > > jmp .Ljump_to_C_code > > > SYM_CODE_END(start_cpu0) > > > > > > but that would only be possible be after more cleanups which converge > > > cpu_bringup_and_idle() with start_secondary(). > > > > > > > The attached patch seems to work fine. > > > > > > The patch looks good to me. > > > > > > It doesn't solve Paul's original issue where arch_cpu_idle_dead() needs > > > to be __noreturn. But that should probably be a separate patch anyway. > > > > Okay, I'll split this off. > > > > > > > > > The __noreturn annotation seems to trigger an objtool warning, though, in > > > > spite of the added BUG() at the end of xen_pv_play_dead(): > > > > > > > > arch/x86/xen/smp_pv.o: warning: objtool: xen_pv_play_dead() falls through to > > > > next function xen_pv_cpu_die() > > > > > > You'll need to tell objtool that xen_cpu_bringup_again() is noreturn by > > > adding "xen_cpu_bringup_again" to global_noreturns[] in > > > tools/objtool/check.c. > > > > Ah, okay. Will do that. > > > > > (Yes it's a pain, I'll be working an improved solution to the noreturn > > > thing...) > > > > Should be fairly easy, no? > > > > "Just" extend the __noreturn macro to put the function into a ".text.noreturn" > > section, which can be handled in a special way by objtool. This would need > > an __init_noreturn macro, of course, for a ".init.text.noreturn" section. > > And in last night's -next run, that diagnostic was gone! > > But of course another appeared in its place: > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: check_relocations+0xd1: stack state mismatch: cfa1=4+32 cfa2=-1+0 > > The .config file is shown below. Thoughts? And it is back. Which makes no sense, but there it is. Thanx, Paul
Powered by blists - more mailing lists