linux-kernel - Re: [patch 4/8] x86/entry: Move irq tracing on syscall entry to C-code

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: <87lfolfo79.fsf@nanos.tec.linutronix.de>
Date:   Sat, 29 Feb 2020 15:44:10 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Andy Lutomirski <luto@...capital.net>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Andy Lutomirski <luto@...nel.org>,
        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>,
        "Paul E. McKenney" <paulmck@...nel.org>
Subject: Re: [patch 4/8] x86/entry: Move irq tracing on syscall entry to C-code

Thomas Gleixner <tglx@...utronix.de> writes:
> Andy Lutomirski <luto@...capital.net> writes:
>>> On Feb 26, 2020, at 12:17 AM, Peter Zijlstra <peterz@...radead.org> wrote:
>>> On Tue, Feb 25, 2020 at 09:43:46PM -0800, Andy Lutomirski wrote:
>>>> Your earlier patches suggest quite strongly that tracing isn't safe
>>>> until enter_from_user_mode().  But trace_hardirqs_off() calls
>>>> trace_irq_disable_rcuidle(), which looks [0] like a tracepoint.
>>>> 
>>>> Did you perhaps mean to do this *after* enter_from_user_mode()?
>>> 
>>> aside from the fact that enter_from_user_mode() itself also has a
>>> tracepoint, the crucial detail is that we must not trace/kprobe the
>>> function calling this.
>>> 
>>> Specifically for #PF, because we need read_cr2() before this. See later
>>> patches.
>>
>> Indeed. I’m fine with this patch, but I still don’t understand what
>> the changelog is about.
>
> Yeah, the changelog is not really helpful. Let me fix that.
>
>> And I’m still rather baffled by most of the notrace annotations in the
>> series.
>
> As discussed on IRC, this might be too broad, but then I rather have the
> actual C-entry points neither traceable nor probable in general and
> relax this by calling functions which can be traced and probed.
>
> My rationale for this decision was that enter_from_user_mode() is marked
> notrace/noprobe as well, so I kept the protection scope the same as we
> had in the ASM maze which is marked noprobe already.

I have second thoughts vs. tracing in this context.

While the tracer itself seems to handle this correctly, what about
things like BPF programs which can be attached to tracepoints and
function trace entries?

Is that really safe _before_ context tracking has updated RCU state?

Thanks,

        tglx