From: Linus Torvalds
> Sent: 19 July 2022 01:02
> 
> On Mon, Jul 18, 2022 at 4:52 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Honestly, I think that would be a better model - yes, you lose 8 bits
> > of hash, but considering that apparently the current KCFI code
> > *guarantees* that the hash pattern will exist even outside the actual
> > target pattern,
> 
> Gaah, I'm being stupid,. You still get the value collision, since the
> int3 byte pattern would just be part of the compare pattern.
> 
> You'd have to use some multi-instruction compare to avoid having the
> pattern in the instruction stream. Probably with another register.
> Like
> 
>         movl -FIXED_OFFSET(%eax),%rdx
>         addl $ANTI_PATTERN,%rdx
>         je ok
> 
> so that the "compare" wouldn't use the same pattern value, but be an
> add with the negated pattern value instead.
> 
> The extra instruction is likely less of a problem than the extra register used.

Shouldn't it be testing the value the caller supplied?

The extra instruction is likely to be one clock - I doubt it will
sensibly run in parallel with code later in the function.

The larger costs are (probably) polluting the D$ with I addresses
(already done by the caller) and the likely mispredicted 'je ok'.
Unless the function has been recently called the 'je ok' gets
static prediction.
While traditionally that would predict a forwards branch 'not taken'
ISTR more recent Intel cpu just use the predictor output - ie random.
Not at all sure about AMD cpu though.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

On Tue, Jul 19, 2022 at 1:26 AM David Laight <David.Laight@aculab.com> wrote:
>
> Shouldn't it be testing the value the caller supplied?

Actually, I'm just all confused.

All that verification code is *in* the caller, before the call - to
verify that the target looks fine.

I think I was confused by the hash thunk above the function also being
generated with a "cmpl $hash". And I don't even know why that is, and
why it wasn't just the bare constant.

            Linus

On Tue, Jul 19, 2022 at 09:27:02AM -0700, Linus Torvalds wrote:
> On Tue, Jul 19, 2022 at 1:26 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > Shouldn't it be testing the value the caller supplied?
> 
> Actually, I'm just all confused.
> 
> All that verification code is *in* the caller, before the call - to
> verify that the target looks fine.
> 
> I think I was confused by the hash thunk above the function also being
> generated with a "cmpl $hash". And I don't even know why that is, and
> why it wasn't just the bare constant.

The preamble hash is encoded into an instruction just to avoid special
casing objtool, which would otherwise get confused about the random
bytes. On arm64, we just emit a bare constant before the function.

Sami

On Tue, Jul 19, 2022 at 10:23 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> The preamble hash is encoded into an instruction just to avoid special
> casing objtool, which would otherwise get confused about the random
> bytes. On arm64, we just emit a bare constant before the function.

Ahh.

I think objtool would want to understand about kCFI anyway, so I think
in the long run that hack isn't a goog idea.

But I get why you'd do it as a "do this as just a compiler thing and
hide it from objtool" as a development strategy.

                Linus

On Tue, Jul 19, 2022 at 10:27:00AM -0700, Linus Torvalds wrote:
> On Tue, Jul 19, 2022 at 10:23 AM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> > The preamble hash is encoded into an instruction just to avoid special
> > casing objtool, which would otherwise get confused about the random
> > bytes. On arm64, we just emit a bare constant before the function.
> 
> Ahh.
> 
> I think objtool would want to understand about kCFI anyway, so I think
> in the long run that hack isn't a goog idea.
> 
> But I get why you'd do it as a "do this as just a compiler thing and
> hide it from objtool" as a development strategy.

I believe it was actually Peter's idea to use an instruction. :) In
earlier revisions of KCFI, I did teach objtool about the preambles, but
that was just so it can ignore them.

Sami

On Tue, Jul 19, 2022 at 11:06:40AM -0700, Sami Tolvanen wrote:
> On Tue, Jul 19, 2022 at 10:27:00AM -0700, Linus Torvalds wrote:
> > On Tue, Jul 19, 2022 at 10:23 AM Sami Tolvanen <samitolvanen@google.com> wrote:
> > >
> > > The preamble hash is encoded into an instruction just to avoid special
> > > casing objtool, which would otherwise get confused about the random
> > > bytes. On arm64, we just emit a bare constant before the function.
> > 
> > Ahh.
> > 
> > I think objtool would want to understand about kCFI anyway, so I think
> > in the long run that hack isn't a goog idea.
> > 
> > But I get why you'd do it as a "do this as just a compiler thing and
> > hide it from objtool" as a development strategy.
> 
> I believe it was actually Peter's idea to use an instruction. :) In
> earlier revisions of KCFI, I did teach objtool about the preambles, but
> that was just so it can ignore them.

Right; even if we teach objtool about kCFI, having text be actual
instructions makes things much nicer. Objdump and friends also shit
their pants if you put random bytes in. It only costs a single byte to
encode the immediate, so why not.

Specifically, the encoding used is:

	movl $0x12345678, %eax

and that is 0xb8 followed by the constant, but there's plenty other
single byte ops that could be used.