linux-kernel - Re: [PATCH v2 02/14] x86/alternatives: Optimize optimize

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: <20210322110734.GC6481@zn.tnic>
Date:   Mon, 22 Mar 2021 12:07:34 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, jpoimboe@...hat.com, jgross@...e.com,
        mbenes@...e.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 02/14] x86/alternatives: Optimize optimize_nops()

On Mon, Mar 22, 2021 at 09:17:57AM +0100, Peter Zijlstra wrote:
> You've make it only replace a single stream of NOPs. Which is probably
> fine, but... :-)

Yap, since I added the padding thing there should be no need to put NOPs
in the middle.

> So mine, while a little more complicated, will replace any number of NOP

A little? I had to sprinkle printks to figure out what it does :)

And what it does is, it calls the instruction decoder on every byte and
advances by the length of each insn it decoded. But that is unnecessary
because the possible opcode bytes you should get to see in any possible
location are:

[(insn).* NOP*]

i.e., 0 or more instructions which are non-NOPs followed by 0 or more
NOPs. Thus my simpler solution to scan past the non-NOPs and patch the
rest.

So I don't see the need for the complexity, frankly.

Btw, yours needs some adjusting to the DUMP_BYTES indices call:

[    0.145789] SMP alternatives: SKIP feat: 8*32+16, old: (entry_SYSCALL_64_after_hwframe+0x49/0xc5 (ffffffff81a0006d) len: 23), repl: (ffffffff89764c3f, len: 23)
[    0.146699] SMP alternatives: ffffffff81a0006d:		 old_insn: 48 c7 c0 90 90 00 00 90 90 90 cd 30 90 90 90 90 90 90 90 90 90 90 90
[    0.148121] SMP alternatives: ffffffff89764c3f: 		 rpl_insn: 48 bb ef be ad de 00 00 00 00 48 31 c0 48 b9 be ba ee ff c0 00 00 00
[    0.149405] SMP alternatives: ffffffff81a0006d:   [7:3) optimized NOPs: 48 c7 c0 90 90 00 00 0f 1f 00 cd 30 90 90 90 90 90 90 90 90 90 90 90
[    0.150700] SMP alternatives: ffffffff81a0006d: [11:11) optimized NOPs: 48 c7 c0 90 90 00 00 0f 1f 00 cd 0f 1f 84 00 00 00 00 00 0f 1f 00 90

[7:3) and [11:11) look weird.

The thing I'm testing with is:

        ALTERNATIVE "mov $0x9090, %rax; .byte 0x90,0x90,0x90; int $0x30", "mov $0xdeadbeef, %rbx; xor %rax, %rax; mov $0xc0ffeebabe, %rcx", \
                X86_FEATURE_XENPV

which is arbitrary, ofc.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette