On Mon, Feb 13, 2023 at 03:31:21PM -0800, Sami Tolvanen wrote: > On Mon, Feb 13, 2023 at 2:17 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Mon, Feb 13, 2023 at 12:49 PM Nick Desaulniers > > <ndesaulniers@xxxxxxxxxx> wrote: > > > > > > I haven't looked at the series in question, but note that this compile > > > time warning is meant to help us catch Control Flow Integrity runtime > > > violations, which may result in a panic. > > Here's the tracking issue for the other warnings of this type in the > kernel, nearly all the warnings are in one driver: > > https://github.com/ClangBuiltLinux/linux/issues/1724 > > > It's a transition from kernel to bpf prog. > > If CFI trips on it it will trip on all transitions. > > All calls from kernel into bpf are more or less the same. > > Not sure what it means for other archs, but on x86 JIT emits 'endbr' > > insn to make IBT/CFI happy. > > While IBT allows indirect calls to any function that starts with > endbr, CFI is more fine-grained and requires the function pointer type > to match the function type, which further limits possible call > targets. To actually enforce this, the compiler emits a type hash > prefix for each function, and a check before each indirect call to > ensure the call target has the expected prefix. The commit message > here has an example of the code the compiler generates: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3c516f89e17e56b4738f05588e51267e295b5e63 Another good commit to look at is: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=931ab63664f02b17d2213ef36b83e1e50190a0aa That includes the FineIBT variant too. > As calling a JIT compiled function would obviously trip CFI, indirect > call checking is currently disabled in BPF dispatcher functions (using > the __nocfi attribute). However, BPF trampolines still have the same > problem, I believe. I wouldn't mind coming up with a solution for > CFI+BPF JIT compatibility, but I haven't had much time to look into > this. Basically, in places where we currently emit an endbr > instruction, we should also emit a type hash prefix. > > Generating type prefixes for functions called through a dispatcher > shouldn't be that hard because the function type is always the same, > but figuring out the correct type for indirect calls that don't go > through a dispatcher might not be entirely trivial, although I'm sure > the BPF verifier/compiler must have this information. FineIBT also > complicates matters a bit here as the actual prefix format differs > from the basic KCFI prefix. > > I'm not sure if Peter or Joao have had time to look at this, but I > would be happy to hear any suggestions you might have. I've not had time to look at this -- but afair BPF will only emit an indirect jump (tail-call even, irrc), it doesn't do indirect calls otherwise (this is also the only place we have RETPOLINE magic in the JIT). (ooh, also dispatcher thing) This generates an unadorned indirect jump, that is, it doesn't have any kCFI magic included, eg. traditional calling convention. The other case, which you allude to I think, is control transfer to the JIT'ed code which is currently __nocfi annotated. I've only briefly thought about how to change this, but basically it would entail the JIT emitting the correct prefix bytes and setting the entry point at +16. Given the JIT will only run after we've selected kCFI/FineIBT it knows which form to pick from and can emit the right prefix. Then if we remove the __nocfi annotation from the call into JIT'ed code, everything should work. None of this should be exceptionally hard, but I've not had time to actually do much about it yet.
