linux-kernel - Re: [patch V4 part 2 10/18] x86/entry/64: Check IF in __preempt_enable

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: <87k11j4ogb.fsf@nanos.tec.linutronix.de>
Date:   Sun, 10 May 2020 20:47:16 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Alexandre Chartre <alexandre.chartre@...cle.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>,
        Brian Gerst <brgerst@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Will Deacon <will@...nel.org>
Subject: Re: [patch V4 part 2 10/18] x86/entry/64: Check IF in __preempt_enable_notrace() thunk

Thomas Gleixner <tglx@...utronix.de> writes:
> Andy Lutomirski <luto@...nel.org> writes:
>> On Tue, May 5, 2020 at 7:14 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>> /**
>>  * preempt_schedule_notrace - preempt_schedule called by tracing
>>  *
>>  * The tracing infrastructure uses preempt_enable_notrace to prevent
>>  * recursion and tracing preempt enabling caused by the tracing
>>  * infrastructure itself. But as tracing can happen in areas coming
>>  * from userspace or just about to enter userspace, a preempt enable
>>  * can occur before user_exit() is called. This will cause the scheduler
>>  * to be called when the system is still in usermode.
>>  *
>>  * To prevent this, the preempt_enable_notrace will use this function
>>  * instead of preempt_schedule() to exit user context if needed before
>>  * calling the scheduler.
>>  */
>>
>> Which is no longer really applicable to x86 -- in the state that this
>> comment nonsensically refers to as "userspace", x86 *always* has IRQs
>> off, which means that preempt_enable() will not schedule.

Yeah.

>> So I'm guessing that the issue you're solving is that we have
>> redundant preempt disable/enable pairs somewhere in the bowels of
>> tracing code that is called with IRQs off, and objtool is now
>> complaining.  Could the actual code in question be fixed to assert
>> that IRQs are off instead of disabling preemption?  If not, can you
>> fix the $SUBJECT and changelog and perhaps add a comment to the code
>> as to *why* you're checking IF?  Otherwise some intrepid programmer is
>> going to notice it down the road, wonder if it's optimizing anything
>> useful at all, and get rid of it.
>
> Let me stare into that again.

There are a few preempt_disable/enable() pairs in some of the helper
functions which are called in various places. That means we would have
to chase all of them and provide 'naked' helpers for these particular
call chains. I'll fix the changelog and add a comment to make clear what
this is about.

Thanks,

        tglx