On Sat, Jan 4, 2025 at 3:28 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Tue, Dec 24, 2024 at 7:25 PM Menglong Dong <menglong8.dong@xxxxxxxxx> wrote: > > > > On Fri, Dec 20, 2024 at 10:01 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > > > > > On Fri, Dec 20, 2024 at 09:57:22PM +0800, Menglong Dong wrote: > > > > > > > However, the other 5-bytes will be consumed if CFI_CLANG is > > > > enabled, and the space is not enough anymore in this case, and > > > > the insn will be like this: > > > > > > > > __cfi_do_test: > > > > mov (5byte) > > > > nop nop (2 bytes) > > > > sarq (9 bytes) > > > > do_test: > > > > xxx > > > > > > > > > > FineIBT will fully consume those 16 bytes. > > > > > > Also, text is ROX, you cannot easily write there. Furthermore, writing > > > non-instructions there will destroy disassemblers ability to make sense > > > of the memory. > > > > Thanks for the reply. Your words make sense, and it > > seems to be dangerous too. > > Raw bytes are indeed dangerous in the text section, but > I think we can make it work. > > We can prepend 5 byte mov %eax, 0x12345678 > or 10 byte mov %rax, 0x12345678aabbccdd > instructions before function entry and before FineIBT/kcfi preamble. Sounds great, which makes the raw bytes safe enough! > Ideally 5 byte insn and use 4 byte as an offset within 4Gb region > for this per-function metadata that we will allocate on demand. > We can prototype with 10 byte insn and full 8 byte pointer to metadata. > Without mitigations it will be > -fpatchable-function-entry=10 > with FineIBT > -fpatchable-function-entry=26 Yeah! And we even don't need extra bytes if CONFIG_CFI_CLANG is not enabled and mitigation is enabled (which is enabled by default), as the remaining 7-bytes is enough for us. > but we have to measure the impact on I-cache iTLB first. > > Menglong, > could you do performance benchmarking for no-mitigation kernel > with extra 5 and extra 10 bytes of padding ? Okay, I'll do this performance benchmarking (maybe a few days later, as I am busy at my work these days :/). > > Since we have: > select FUNCTION_ALIGNMENT_16B if X86_64 || X86_ALIGNMENT_16 > > the functions are aligned to 16 all the time, > so there is some gap between them. > Extra -fpatchable-function-entry=5 might be in the noise > from performance point of view, > but the ability to provide such per function metadata block > will be very useful for all kinds of use cases. Thanks for the advice, which gives me some faith in this idea. Thanks! Menglong Dong
