linux-kernel - Re: [patch 13/16] x86/entry: Move irqflags and context tracking to C for simple idtentries

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]

Message-Id: <2A899107-5AB8-4907-8AF2-31154A2E0A98@amacapital.net>
Date:   Wed, 26 Feb 2020 09:09:34 -0800
From:   Andy Lutomirski <luto@...capital.net>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Brian Gerst <brgerst@...il.com>,
        Juergen Gross <JGross@...e.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [patch 13/16] x86/entry: Move irqflags and context tracking to C for simple idtentries



> On Feb 26, 2020, at 9:05 AM, Frederic Weisbecker <frederic@...nel.org> wrote:
> 
> On Wed, Feb 26, 2020 at 09:05:38AM +0100, Peter Zijlstra wrote:
>>> On Tue, Feb 25, 2020 at 11:33:34PM +0100, Thomas Gleixner wrote:
>>> 
>>> --- a/arch/x86/include/asm/idtentry.h
>>> +++ b/arch/x86/include/asm/idtentry.h
>>> @@ -7,14 +7,31 @@
>>> 
>>> #ifndef __ASSEMBLY__
>>> 
>>> +#ifdef CONFIG_CONTEXT_TRACKING
>>> +static __always_inline void enter_from_user_context(void)
>>> +{
>>> +    CT_WARN_ON(ct_state() != CONTEXT_USER);
>>> +    user_exit_irqoff();
>>> +}
>>> +#else
>>> +static __always_inline void enter_from_user_context(void) { }
>>> +#endif
>>> +
>>> /**
>>>  * idtentry_enter - Handle state tracking on idtentry
>>>  * @regs:    Pointer to pt_regs of interrupted context
>>>  *
>>> - * Place holder for now.
>>> + * Invokes:
>>> + *  - The hardirq tracer to keep the state consistent as low level ASM
>>> + *    entry disabled interrupts.
>>> + *
>>> + *  - Context tracking if the exception hit user mode
>>>  */
>>> static __always_inline void idtentry_enter(struct pt_regs *regs)
>>> {
>>> +    trace_hardirqs_off();
>>> +    if (user_mode(regs))
>>> +        enter_from_user_context();
>>> }
>> 
>> So:
>> 
>>    asm_exc_int3
>>      exc_int3
>>        idtentry_enter()
>>          enter_from_user_context
>>            if (context_tracking_enabled())
>> 
>>        poke_int3_handler();
>> 
>> Is, AFAICT, completely buggered.
>> 
>> You can't have a static_branch before the poke_int3_handler that deals
>> with text_poke.
> 
> #BP is treated like an NMI in your patchset IIRC?
> In that case and since that implies we can't schedule, we can remove
> the call to context tracking there once we have nmi_enter()/nmi_exit()
> called unconditionally.

int3 from user mode can send signals. This has better be able to schedule by the time it hits prepare_exit_to_usermode.