linux-kernel - Re: [PATCH v4 01/13] objtool: Remove CFI save/restore special case

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]

Date:   Thu, 26 Mar 2020 16:49:38 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     tglx@...utronix.de, linux-kernel@...r.kernel.org, x86@...nel.org,
        mhiramat@...nel.org, mbenes@...e.cz
Subject: Re: [PATCH v4 01/13] objtool: Remove CFI save/restore special case

On Thu, Mar 26, 2020 at 08:56:20AM -0500, Josh Poimboeuf wrote:
> On Thu, Mar 26, 2020 at 12:30:49PM +0100, Peter Zijlstra wrote:
> > This isn't ideal either; stuffing code with NOPs just to make
> > annotations work is certainly sub-optimal, but given that sync_core()
> > is stupid expensive in any case, one extra nop isn't going to be a
> > problem here.
> 
> /me puts his hardened objtool maintainer's glasses on...
> 
> The problem is, when you do this kind of change to somebody else's code
> -- like adding a NOP to driver code -- there's a 90% chance they'll NACK
> it and tell you to fix your shit.  Because they'll be happy to tell you
> the code itself should never be changed just to "make objtool happy".

So the only objtool annotation drivers tend to run into is that uaccess
crud. Drivers really had better not need the CFI annotations, otherwise
they doing massively dodgy things.

And the nice thing with the uaccess crud is that they're anchored to
actual instructions, as opposed to this.

> And honestly, they'd be right, and there's not much you can say in
> reply.  And then we end up having to fix it in objtool again anyway.

Well, I agree. I just haven't managed to come up with anything sensible.

> The 'insn == first' check isn't ideal, but at least it works (I think?).

It works, yes, for exactly this one case.