[v3,0/7] x86/cfi,bpf: Fix CFI vs eBPF

Message

Peter Zijlstra Dec. 15, 2023, 9:12 a.m. UTC

Hi!

What started with the simple observation that bpf_dispatcher_*_func() was
broken for calling CFI functions with a __nocfi calling context for FineIBT
ended up with a complete BPF wide CFI fixup.

With these changes on the BPF selftest suite passes without crashing -- there's
still a few failures, but Alexei has graciously offered to look into those.

(Alexei, I have presumed your SoB on the very last patch, please update
as you see fit)

Changes since v2 are numerous but include:
 - cfi_get_offset() -- as a means to communicate the offset (ast)
 - 5 new patches fixing various BPF internals to be CFI clean

Note: it *might* be possible to merge the
bpf_bpf_tcp_ca.c:unsupported_ops[] thing into the CFI stubs, as is
get_info will have a NULL stub, unlike the others.

---
 arch/riscv/include/asm/cfi.h   |   3 +-
 arch/riscv/kernel/cfi.c        |   2 +-
 arch/x86/include/asm/cfi.h     | 126 +++++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/alternative.c  |  87 +++++++++++++++++++++++---
 arch/x86/kernel/cfi.c          |   4 +-
 arch/x86/net/bpf_jit_comp.c    | 134 +++++++++++++++++++++++++++++++++++------
 include/asm-generic/Kbuild     |   1 +
 include/linux/bpf.h            |  27 ++++++++-
 include/linux/cfi.h            |  12 ++++
 kernel/bpf/bpf_struct_ops.c    |  16 ++---
 kernel/bpf/core.c              |  25 ++++++++
 kernel/bpf/cpumask.c           |   8 ++-
 kernel/bpf/helpers.c           |  18 +++++-
 net/bpf/bpf_dummy_struct_ops.c |  31 +++++++++-
 net/bpf/test_run.c             |  15 ++++-
 net/ipv4/bpf_tcp_ca.c          |  69 +++++++++++++++++++++
 16 files changed, 528 insertions(+), 50 deletions(-)

Comments

[e-mail] Dec. 16, 2023, 12:50 a.m. UTC | #1

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Fri, 15 Dec 2023 10:12:16 +0100 you wrote:
> Hi!
> 
> What started with the simple observation that bpf_dispatcher_*_func() was
> broken for calling CFI functions with a __nocfi calling context for FineIBT
> ended up with a complete BPF wide CFI fixup.
> 
> With these changes on the BPF selftest suite passes without crashing -- there's
> still a few failures, but Alexei has graciously offered to look into those.
> 
> [...]

Here is the summary with links:
  - [v3,1/7] cfi: Flip headers
    https://git.kernel.org/bpf/bpf-next/c/4382159696c9
  - [v3,2/7] x86/cfi,bpf: Fix BPF JIT call
    https://git.kernel.org/bpf/bpf-next/c/4f9087f16651
  - [v3,3/7] x86/cfi,bpf: Fix bpf_callback_t CFI
    https://git.kernel.org/bpf/bpf-next/c/e72d88d18df4
  - [v3,4/7] x86/cfi,bpf: Fix bpf_struct_ops CFI
    https://git.kernel.org/bpf/bpf-next/c/2cd3e3772e41
  - [v3,5/7] cfi: Add CFI_NOSEAL()
    https://git.kernel.org/bpf/bpf-next/c/e9d13b9d2f99
  - [v3,6/7] bpf: Fix dtor CFI
    https://git.kernel.org/bpf/bpf-next/c/e4c00339891c
  - [v3,7/7] x86/cfi,bpf: Fix bpf_exception_cb() signature
    https://git.kernel.org/bpf/bpf-next/c/852486b35f34

You are awesome, thank you!

Alexei Starovoitov Dec. 16, 2023, 12:53 a.m. UTC | #2

On Fri, Dec 15, 2023 at 1:33 AM Peter Zijlstra <[e-mail]> wrote:
>
> Hi!
>
> What started with the simple observation that bpf_dispatcher_*_func() was
> broken for calling CFI functions with a __nocfi calling context for FineIBT
> ended up with a complete BPF wide CFI fixup.
>
> With these changes on the BPF selftest suite passes without crashing -- there's
> still a few failures, but Alexei has graciously offered to look into those.
>
> (Alexei, I have presumed your SoB on the very last patch, please update
> as you see fit)
>
> Changes since v2 are numerous but include:
>  - cfi_get_offset() -- as a means to communicate the offset (ast)
>  - 5 new patches fixing various BPF internals to be CFI clean

Looks great to me. Pushed to bpf-next.

There is a failure on s390 that I temporarily denylisted
with an extra patch.
And sent a proposed fix:
https://lore.kernel.org/bpf/[e-mail]/

Ilya,
please take a look.

> Note: it *might* be possible to merge the
> bpf_bpf_tcp_ca.c:unsupported_ops[] thing into the CFI stubs, as is
> get_info will have a NULL stub, unlike the others.

That's a good idea. Will clean up unsupported_ops.
Either myself or Martin will follow up.