linux-kernel - Re: objtool warning for next-20221118

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 25 Nov 2022 06:30:35 +0100
From:   Juergen Gross <jgross@...e.com>
To:     Josh Poimboeuf <jpoimboe@...nel.org>
Cc:     Andrew Cooper <Andrew.Cooper3@...rix.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "Paul E. McKenney" <paulmck@...nel.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 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.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)