BPF List
 help / color / mirror / Atom feed
* [PATCH bpf v2 0/8] bpf: track changes_pkt_data property for global functions
@ 2024-12-10  4:10 Eduard Zingerman
  2024-12-10  4:10 ` [PATCH bpf v2 1/8] bpf: add find_containing_subprog() utility function Eduard Zingerman
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Eduard Zingerman @ 2024-12-10  4:10 UTC (permalink / raw)
  To: bpf, ast
  Cc: andrii, daniel, martin.lau, kernel-team, yonghong.song, mejedi,
	Eduard Zingerman

Nick Zavaritsky reported [0] a bug in verifier, where the following
unsafe program is not rejected:

    __attribute__((__noinline__))
    long skb_pull_data(struct __sk_buff *sk, __u32 len)
    {
        return bpf_skb_pull_data(sk, len);
    }

    SEC("tc")
    int test_invalidate_checks(struct __sk_buff *sk)
    {
        int *p = (void *)(long)sk->data;
        if ((void *)(p + 1) > (void *)(long)sk->data_end) return TCX_DROP;
        skb_pull_data(sk, 0);
        /* not safe, p is invalid after bpf_skb_pull_data call */
        *p = 42;
        return TCX_PASS;
    }

This happens because verifier does not track package invalidation
effect of global sub-programs.

This patch-set fixes the issue by modifying check_cfg() to compute
whether or not each sub-program calls (directly or indirectly)
helper invalidating packet pointers.

As global functions could be replaced with extension programs,
a new field 'changes_pkt_data' is added to struct bpf_prog_aux.
Verifier only allows replacing functions that do not change packet
data with functions that do not change packet data.

In case if there is a need to a have a global function that does not
change packet data, but allow replacing it with function that does,
the recommendation is to add a noop call to a helper, e.g.:
- for skb do 'bpf_skb_change_proto(skb, 0, 0)';
- for xdp do 'bpf_xdp_adjust_meta(xdp, 0)'.

Functions also can do tail calls. Effects of the tail call cannot be
analyzed before-hand, thus verifier assumes that tail calls always
change packet data.

Changes v1 [1] -> v2:
- added handling of extension programs and tail calls
  (thanks, Alexei, for all the input).

[0] https://lore.kernel.org/bpf/0498CA22-5779-4767-9C0C-A9515CEA711F@gmail.com/
[1] https://lore.kernel.org/bpf/20241206040307.568065-1-eddyz87@gmail.com/

Eduard Zingerman (8):
  bpf: add find_containing_subprog() utility function
  bpf: refactor bpf_helper_changes_pkt_data to use helper number
  bpf: track changes_pkt_data property for global functions
  selftests/bpf: test for changing packet data from global functions
  bpf: check changes_pkt_data property for extension programs
  selftests/bpf: freplace tests for tracking of changes_packet_data
  bpf: consider that tail calls invalidate packet pointers
  selftests/bpf: validate that tail call invalidates packet pointers

 include/linux/bpf.h                           |  1 +
 include/linux/bpf_verifier.h                  |  1 +
 include/linux/filter.h                        |  2 +-
 kernel/bpf/core.c                             |  2 +-
 kernel/bpf/verifier.c                         | 78 ++++++++++++++++---
 net/core/filter.c                             | 65 +++++++---------
 .../bpf/prog_tests/changes_pkt_data.c         | 76 ++++++++++++++++++
 .../selftests/bpf/progs/changes_pkt_data.c    | 26 +++++++
 .../bpf/progs/changes_pkt_data_freplace.c     | 18 +++++
 .../testing/selftests/bpf/progs/tc_bpf2bpf.c  |  2 +
 .../selftests/bpf/progs/verifier_sock.c       | 56 +++++++++++++
 11 files changed, 280 insertions(+), 47 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/changes_pkt_data.c
 create mode 100644 tools/testing/selftests/bpf/progs/changes_pkt_data.c
 create mode 100644 tools/testing/selftests/bpf/progs/changes_pkt_data_freplace.c

-- 
2.47.0


^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-12-10 19:06 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-10  4:10 [PATCH bpf v2 0/8] bpf: track changes_pkt_data property for global functions Eduard Zingerman
2024-12-10  4:10 ` [PATCH bpf v2 1/8] bpf: add find_containing_subprog() utility function Eduard Zingerman
2024-12-10  4:10 ` [PATCH bpf v2 2/8] bpf: refactor bpf_helper_changes_pkt_data to use helper number Eduard Zingerman
2024-12-10  4:10 ` [PATCH bpf v2 3/8] bpf: track changes_pkt_data property for global functions Eduard Zingerman
2024-12-10  4:10 ` [PATCH bpf v2 4/8] selftests/bpf: test for changing packet data from " Eduard Zingerman
2024-12-10  4:10 ` [PATCH bpf v2 5/8] bpf: check changes_pkt_data property for extension programs Eduard Zingerman
2024-12-10  4:10 ` [PATCH bpf v2 6/8] selftests/bpf: freplace tests for tracking of changes_packet_data Eduard Zingerman
2024-12-10  4:10 ` [PATCH bpf v2 7/8] bpf: consider that tail calls invalidate packet pointers Eduard Zingerman
2024-12-10 10:35   ` Nick Zavaritsky
2024-12-10 18:23     ` Alexei Starovoitov
2024-12-10 18:29       ` Eduard Zingerman
2024-12-10 18:31         ` Alexei Starovoitov
2024-12-10 18:52           ` Eduard Zingerman
2024-12-10 19:00             ` Alexei Starovoitov
2024-12-10 19:06               ` Eduard Zingerman
2024-12-10  4:11 ` [PATCH bpf v2 8/8] selftests/bpf: validate that tail call invalidates " Eduard Zingerman
2024-12-10 18:40 ` [PATCH bpf v2 0/8] bpf: track changes_pkt_data property for global functions patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox