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@xxxxxxxxx/
[1] https://lore.kernel.org/bpf/20241206040307.568065-1-eddyz87@xxxxxxxxx/
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
