Messages in this thread | | | Date | Sun, 10 Sep 2023 16:42:27 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v2 10/11] x86/alternatives: Simplify ALTERNATIVE_n() |
| |
On Sat, Sep 09, 2023 at 11:25:54AM +0200, Peter Zijlstra wrote: > So what you end up with is: > > 661: > "one byte orig insn" > "one nop because alt1 is 2 bytes" > "one nop because alt2 is 3 bytes" > > right?
Right.
> This becomes more of a problem with your example above where the > respective lengths are 0, 5, 16. In that case, when you patch 5, you'll > leave 11 single nops in there.
Well, I know what you mean but the code handles that gracefully and it works. Watch this:
I made it always apply the second one and not apply the third, the longest one:
.macro UNTRAIN_RET #if defined(CONFIG_CPU_UNRET_ENTRY) || defined(CONFIG_CPU_IBPB_ENTRY) || \ defined(CONFIG_CALL_DEPTH_TRACKING) || defined(CONFIG_CPU_SRSO) VALIDATE_UNRET_END ALTERNATIVE_3 "", \ CALL_UNTRAIN_RET, X86_FEATURE_UNRET, \ "call entry_ibpb", X86_FEATURE_ALWAYS, \ __stringify(RESET_CALL_DEPTH), X86_FEATURE_CALL_DEPTH #endif .endm
So it comes in and pokes in the padding for the first one: X86_FEATURE_UNRET
[ 0.903506] SMP alternatives: feat: 11*32+15, old: (entry_SYSCALL_64_after_hwframe+0x59/0xd8 (ffffffff81c000d1) len: 5), repl: (ffffffff833a362b, len: 5) [ 0.911256] SMP alternatives: ffffffff81c000d1: [0:5) optimized NOPs: 0f 1f 44 00 00
Then patches in the entry_ibpb call:
[ 0.916849] SMP alternatives: feat: 3*32+21, old: (entry_SYSCALL_64_after_hwframe+0x59/0xd8 (ffffffff81c000d1) len: 5), repl: (ffffffff833a3630, len: 5) [ 0.924844] SMP alternatives: ffffffff81c000d1: old_insn: 0f 1f 44 00 00 [ 0.928842] SMP alternatives: ffffffff833a3630: rpl_insn: e8 5b 9e 81 fe [ 0.932849] SMP alternatives: ffffffff81c000d1: final_insn: e8 ba d3 fb ff
and now it comes to the call depth thing which is of size 16:
[ 0.936845] SMP alternatives: feat: 11*32+19, old: (entry_SYSCALL_64_after_hwframe+0x59/0xd8 (ffffffff81c000d1) len: 16), repl: (ffffffff833a3635, len: 16) [ 0.940844] SMP alternatives: __optimize_nops: next: 5, insn len: 5
and this is why it works: __optimize_nops() is cautious enough to do insn_is_nop() and there is the CALL insn: e8 ba d3 fb ff, it skips over it:
[ 0.944852] SMP alternatives: __optimize_nops: next: 6, insn len: 1
next one is a NOP and it patches the rest of it. Resulting in an 11 bytes NOP:
[ 0.950758] SMP alternatives: ffffffff81c000d1: [5:16) optimized NOPs: e8 ba d3 fb ff 66 66 2e 0f 1f 84 00 00 00 00 00
So we're good here without this max(repl_len) thing even if it is the right thing to do.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |