On Fri, Jun 16, 2023 at 09:13:22PM -0700, Andy Lutomirski wrote: > On 5/16/23 14:20, Kent Overstreet wrote: > > On Tue, May 16, 2023 at 02:02:11PM -0700, Kees Cook wrote: > > > For something that small, why not use the text_poke API? > > > > This looks like it's meant for patching existing kernel text, which > > isn't what I want - I'm generating new functions on the fly, one per > > btree node. > > Dynamically generating code is a giant can of worms. > > Kees touched on a basic security thing: a linear address mapped W+X is a big > no-no. And that's just scratching the surface -- ideally we would have a > strong protocol for generating code: the code is generated in some > extra-secure context, then it's made immutable and double-checked, then > it becomes live. "Double checking" arbitrary code is is fantasy. You can't "prove the security" of arbitrary code post compilation. Rice's theorem states that any nontrivial property of a program is either a direct consequence of the syntax, or is undecidable. It's why programs in statically typed languages are easier to reason about, and it's also why the borrow checker in Rust is a syntactic construct. You just have to be able to trust the code that generates the code. Just like you have to be able to trust any other code that lives in kernel space. This is far safer and easier to reason about than what BPF is doing because we're not compiling arbitrary code, the actual codegen part is 200 loc and the input is just a single table. > > (When x86 modifies itself at boot or for static keys, it changes out the > page tables temporarily.) > > And even beyond security, we have correctness. x86 is a fairly forgiving > architecture. If you go back in time about 20 years, modify > some code *at the same linear address at which you intend to execute it*, > and jump to it, it works. It may even work if you do it through > an alias (the manual is vague). But it's not 20 years ago, and you have > multiple cores. This does *not* work with multiple CPUs -- you need to > serialize on the CPU executing the modified code. On all the but the very > newest CPUs, you need to kludge up the serialization, and that's > sloooooooooooooow. Very new CPUs have the SERIALIZE instruction, which > is merely sloooooow. If what you were saying was true, it would be an issue any time we mapped in new executable code for userspace - minor page faults would be stupidly slow. This code has been running on thousands of machines for years, and the only issues that have come up have been due to the recent introduction of indirect branch tracking. x86 doesn't have such broken caches, and architectures that do have utterly broken caches (because that's what you're describing: you're describing caches that _are not coherent across cores_) are not high on my list of things I care about. Also, SERIALIZE is a spectre thing. Not relevant here. > Based on the above, I regret to inform you that jit_update() will either > need to sync all cores via IPI or all cores will need to check whether a > sync is needed and do it themselves. text_poke() doesn't even send IPIs. I think you've been misled about some things :)
