linux-kernel - Re: [patch V9 00/39] x86/entry: Rework leftovers (was part V)

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: <83474edd-195f-f10b-9fe9-8ee168344e29@citrix.com>
Date:   Wed, 3 Jun 2020 20:18:44 +0100
From:   Andrew Cooper <andrew.cooper3@...rix.com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
        "Paul E. McKenney" <paulmck@...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>,
        Tom Lendacky <thomas.lendacky@....com>,
        Wei Liu <wei.liu@...nel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        Jason Chen CJ <jason.cj.chen@...el.com>,
        Zhao Yakui <yakui.zhao@...el.com>
Subject: Re: [patch V9 00/39] x86/entry: Rework leftovers (was part V)

On 22/05/2020 22:17, Peter Zijlstra wrote:
> On Fri, May 22, 2020 at 08:20:15AM +0100, Andrew Cooper wrote:
>> Apologies for opening a related can of worms.
>>
>> The new debug_enter() has propagated a pre-existing issue forward,
>> ultimately caused by bad advice in the SDM.
>>
>> Because the RTM status bit in DR6 has inverted polarity, writing DR6 to
>> 0 causes RTM to appear asserted to any logic which cares, despite RTM
>> debugging not being enabled.  The same is true in principle for what is
>> handed to userspace via u_debugreg[DR_STATUS].
>>
>> On the subject of DR6, the SDM now reads:
>>
>> "Certain debug exceptions may clear bits 0-3. The remaining contents of
>> the DR6 register are never cleared by the processor. To avoid confusion
>> in identifying debug exceptions, debug handlers should clear the
>> register (except bit 16, which they should set) before returning to the
>> interrupted task."
> *URGH*
>
>> First of all, that should read "are never de-asserted by the processor"
>> rather than "cleared", but the advice has still failed to learn from its
>> first mistake.  The forward-compatible way to fix this is to set
>> DR6_DEFAULT (0xffff0ff0) which also covers future inverted polarity bits.
>>
>> As for what to do about userspace, that is harder.  One approach is to
>> express everything in terms of positive polarity (i.e. pass on dr6 ^
>> DR6_DEFAULT), so DR6_RTM only appears set when RTM debugging is
>> enabled.  This approach is already taken with the VMCS PENDING_DBG
>> field, so there is at least previous form.
>>
>> I realise that "do nothing" might be acceptable at this point, given the
>> lack of support for RTM debugging.
> This! I'm thinking "do nothing" is, at this moment, the right thing to
> do. If/when someone goes and tries to make RTM debugging work, they get
> to figure out how to deal with this mess.

Well that didn't last long...

The new ISE (rev 39, published today) introduces BUS LOCK DEBUG
EXCEPTION which is now a second inverted polarity sticky bit (bit 11) in
%dr6.

This one is liable to get more traction than RTM debugging, so something
probably does want fixing in the #DB handler.

~Andrew