Re: [PATCH v4 01/13] objtool: Remove CFI save/restore special case
From: Peter Zijlstra
Date: Thu Mar 26 2020 - 11:49:44 EST
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.