dynptr for skb metadata behaves the same way as the dynptr for skb data with one exception - writes to skb_meta dynptr don't invalidate existing skb and skb_meta slices. Repeat the tests which are specific to skb dynptr for the skb_meta dynptr and add a couple of new tests (skb_data_valid_*) to ensure we don't invalidate the slices in the mentioned case. Signed-off-by: Jakub Sitnicki --- tools/testing/selftests/bpf/prog_tests/dynptr.c | 3 + tools/testing/selftests/bpf/progs/dynptr_fail.c | 297 +++++++++++++++++++++ tools/testing/selftests/bpf/progs/dynptr_success.c | 62 +++++ 3 files changed, 362 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c index f2b65398afce..375962f62f5d 100644 --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c @@ -31,7 +31,9 @@ static struct { {"test_dynptr_memset_xdp_chunks", SETUP_XDP_PROG}, {"test_ringbuf", SETUP_SYSCALL_SLEEP}, {"test_skb_readonly", SETUP_SKB_PROG}, + {"test_skb_meta_readonly", SETUP_SKB_PROG}, {"test_dynptr_skb_data", SETUP_SKB_PROG}, + {"test_dynptr_skb_meta_data", SETUP_SKB_PROG}, {"test_adjust", SETUP_SYSCALL_SLEEP}, {"test_adjust_err", SETUP_SYSCALL_SLEEP}, {"test_zero_size_dynptr", SETUP_SYSCALL_SLEEP}, @@ -41,6 +43,7 @@ static struct { {"test_dynptr_skb_no_buff", SETUP_SKB_PROG}, {"test_dynptr_skb_strcmp", SETUP_SKB_PROG}, {"test_dynptr_skb_tp_btf", SETUP_SKB_PROG_TP}, + {"test_dynptr_skb_meta_tp_btf", SETUP_SKB_PROG_TP}, {"test_probe_read_user_dynptr", SETUP_XDP_PROG}, {"test_probe_read_kernel_dynptr", SETUP_XDP_PROG}, {"test_probe_read_user_str_dynptr", SETUP_XDP_PROG}, diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c index bd8f15229f5c..03c15315360b 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_fail.c +++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c @@ -269,6 +269,26 @@ int data_slice_out_of_bounds_skb(struct __sk_buff *skb) return SK_PASS; } +/* A metadata slice can't be accessed out of bounds */ +SEC("?tc") +__failure __msg("value is outside of the allowed memory range") +int data_slice_out_of_bounds_skb_meta(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *md; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice_rdwr(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + /* this should fail */ + *(md + 1) = 42; + + return SK_PASS; +} + SEC("?raw_tp") __failure __msg("value is outside of the allowed memory range") int data_slice_out_of_bounds_map_value(void *ctx) @@ -1089,6 +1109,26 @@ int skb_invalid_slice_write(struct __sk_buff *skb) return SK_PASS; } +/* bpf_dynptr_slice()s are read-only and cannot be written to */ +SEC("?tc") +__failure __msg("R{{[0-9]+}} cannot write into rdonly_mem") +int skb_meta_invalid_slice_write(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *md; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + /* this should fail */ + *md = 42; + + return SK_PASS; +} + /* The read-only data slice is invalidated whenever a helper changes packet data */ SEC("?tc") __failure __msg("invalid mem access 'scalar'") @@ -1115,6 +1155,29 @@ int skb_invalid_data_slice1(struct __sk_buff *skb) return SK_PASS; } +/* Read-only skb metadata slice is invalidated whenever a helper changes packet data */ +SEC("?tc") +__failure __msg("invalid mem access 'scalar'") +int skb_meta_invalid_data_slice1(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *md; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + if (bpf_skb_pull_data(skb, skb->len)) + return SK_DROP; + + /* this should fail */ + val = *md; + + return SK_PASS; +} + /* The read-write data slice is invalidated whenever a helper changes packet data */ SEC("?tc") __failure __msg("invalid mem access 'scalar'") @@ -1141,6 +1204,29 @@ int skb_invalid_data_slice2(struct __sk_buff *skb) return SK_PASS; } +/* Read-write skb metadata slice is invalidated whenever a helper changes packet data */ +SEC("?tc") +__failure __msg("invalid mem access 'scalar'") +int skb_meta_invalid_data_slice2(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *md; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice_rdwr(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + if (bpf_skb_pull_data(skb, skb->len)) + return SK_DROP; + + /* this should fail */ + *md = 42; + + return SK_PASS; +} + /* The read-only data slice is invalidated whenever bpf_dynptr_write() is called */ SEC("?tc") __failure __msg("invalid mem access 'scalar'") @@ -1167,6 +1253,74 @@ int skb_invalid_data_slice3(struct __sk_buff *skb) return SK_PASS; } +/* Read-only skb metadata slice is invalidated on write to skb data */ +SEC("?tc") +__failure __msg("invalid mem access 'scalar'") +int skb_meta_invalid_data_slice3(struct __sk_buff *skb) +{ + struct bpf_dynptr data, meta; + __u8 *md; + + bpf_dynptr_from_skb(skb, 0, &data); + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + bpf_dynptr_write(&data, 0, "x", 1, 0); + + /* this should fail */ + val = *md; + + return SK_PASS; +} + +/* Read-only skb data slice is _not_ invalidated on write to skb metadata */ +SEC("?tc") +__success +int skb_valid_data_slice3(struct __sk_buff *skb) +{ + struct bpf_dynptr data, meta; + __u8 *d; + + bpf_dynptr_from_skb(skb, 0, &data); + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + d = bpf_dynptr_slice(&data, 0, NULL, sizeof(*d)); + if (!d) + return SK_DROP; + + bpf_dynptr_write(&meta, 0, "x", 1, 0); + + /* this should succeed */ + val = *d; + + return SK_PASS; +} + +/* Read-only skb metadata slice is _not_ invalidated on write to skb metadata */ +SEC("?tc") +__success +int skb_meta_valid_data_slice3(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *md; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + bpf_dynptr_write(&meta, 0, "x", 1, 0); + + /* this should succeed */ + val = *md; + + return SK_PASS; +} + /* The read-write data slice is invalidated whenever bpf_dynptr_write() is called */ SEC("?tc") __failure __msg("invalid mem access 'scalar'") @@ -1192,6 +1346,74 @@ int skb_invalid_data_slice4(struct __sk_buff *skb) return SK_PASS; } +/* Read-write skb metadata slice is invalidated on write to skb data slice */ +SEC("?tc") +__failure __msg("invalid mem access 'scalar'") +int skb_meta_invalid_data_slice4(struct __sk_buff *skb) +{ + struct bpf_dynptr data, meta; + __u8 *md; + + bpf_dynptr_from_skb(skb, 0, &data); + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice_rdwr(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + bpf_dynptr_write(&data, 0, "x", 1, 0); + + /* this should fail */ + *md = 42; + + return SK_PASS; +} + +/* Read-write skb data slice is _not_ invalidated on write to skb metadata */ +SEC("?tc") +__success +int skb_valid_data_slice4(struct __sk_buff *skb) +{ + struct bpf_dynptr data, meta; + __u8 *d; + + bpf_dynptr_from_skb(skb, 0, &data); + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + d = bpf_dynptr_slice_rdwr(&data, 0, NULL, sizeof(*d)); + if (!d) + return SK_DROP; + + bpf_dynptr_write(&meta, 0, "x", 1, 0); + + /* this should succeed */ + *d = 42; + + return SK_PASS; +} + +/* Read-write skb metadata slice is _not_ invalidated on write to skb metadata */ +SEC("?tc") +__success +int skb_meta_valid_data_slice4(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *md; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + md = bpf_dynptr_slice_rdwr(&meta, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + bpf_dynptr_write(&meta, 0, "x", 1, 0); + + /* this should succeed */ + *md = 42; + + return SK_PASS; +} + /* The read-only data slice is invalidated whenever a helper changes packet data */ SEC("?xdp") __failure __msg("invalid mem access 'scalar'") @@ -1255,6 +1477,19 @@ int skb_invalid_ctx(void *ctx) return 0; } +/* Only supported prog type can create skb_meta-type dynptrs */ +SEC("?raw_tp") +__failure __msg("calling kernel function bpf_dynptr_from_skb_meta is not allowed") +int skb_meta_invalid_ctx(void *ctx) +{ + struct bpf_dynptr meta; + + /* this should fail */ + bpf_dynptr_from_skb_meta(ctx, 0, &meta); + + return 0; +} + SEC("fentry/skb_tx_error") __failure __msg("must be referenced or trusted") int BPF_PROG(skb_invalid_ctx_fentry, void *skb) @@ -1267,6 +1502,18 @@ int BPF_PROG(skb_invalid_ctx_fentry, void *skb) return 0; } +SEC("fentry/skb_tx_error") +__failure __msg("must be referenced or trusted") +int BPF_PROG(skb_meta_invalid_ctx_fentry, void *skb) +{ + struct bpf_dynptr meta; + + /* this should fail */ + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + return 0; +} + SEC("fexit/skb_tx_error") __failure __msg("must be referenced or trusted") int BPF_PROG(skb_invalid_ctx_fexit, void *skb) @@ -1279,6 +1526,18 @@ int BPF_PROG(skb_invalid_ctx_fexit, void *skb) return 0; } +SEC("fexit/skb_tx_error") +__failure __msg("must be referenced or trusted") +int BPF_PROG(skb_meta_invalid_ctx_fexit, void *skb) +{ + struct bpf_dynptr meta; + + /* this should fail */ + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + return 0; +} + /* Reject writes to dynptr slot for uninit arg */ SEC("?raw_tp") __failure __msg("potential write to dynptr at off=-16") @@ -1405,6 +1664,22 @@ int invalid_slice_rdwr_rdonly(struct __sk_buff *skb) return 0; } +SEC("cgroup_skb/ingress") +__failure __msg("the prog does not allow writes to packet data") +int invalid_slice_rdwr_rdonly_skb_meta(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *p; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + + /* this should fail */ + p = bpf_dynptr_slice_rdwr(&meta, 0, NULL, sizeof(*p)); + __sink(p); + + return 0; +} + /* bpf_dynptr_adjust can only be called on initialized dynptrs */ SEC("?raw_tp") __failure __msg("Expected an initialized dynptr as arg #0") @@ -1665,6 +1940,28 @@ int clone_skb_packet_data(struct __sk_buff *skb) return 0; } +SEC("?tc") +__failure __msg("invalid mem access 'scalar'") +int clone_skb_packet_meta(struct __sk_buff *skb) +{ + struct bpf_dynptr clone, meta; + __u8 *md; + + bpf_dynptr_from_skb_meta(skb, 0, &meta); + bpf_dynptr_clone(&meta, &clone); + md = bpf_dynptr_slice_rdwr(&clone, 0, NULL, sizeof(*md)); + if (!md) + return SK_DROP; + + if (bpf_skb_pull_data(skb, skb->len)) + return SK_DROP; + + /* this should fail */ + *md = 42; + + return 0; +} + /* A xdp clone's data slices should be invalid anytime packet data changes */ SEC("?xdp") __failure __msg("invalid mem access 'scalar'") diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c index 7d7081d05d47..680c58484cba 100644 --- a/tools/testing/selftests/bpf/progs/dynptr_success.c +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c @@ -188,6 +188,26 @@ int test_skb_readonly(struct __sk_buff *skb) return 1; } +SEC("?cgroup_skb/egress") +int test_skb_meta_readonly(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + int ret; + + err = 1; + ret = bpf_dynptr_from_skb_meta(skb, 0, &meta); + if (ret) + return 1; + + err = 2; + ret = bpf_dynptr_write(&meta, 0, "x", 1, 0); + if (ret != -EINVAL) + return 1; + + err = 0; + return 1; +} + SEC("?cgroup_skb/egress") int test_dynptr_skb_data(struct __sk_buff *skb) { @@ -209,6 +229,27 @@ int test_dynptr_skb_data(struct __sk_buff *skb) return 1; } +SEC("?cgroup_skb/egress") +int test_dynptr_skb_meta_data(struct __sk_buff *skb) +{ + struct bpf_dynptr meta; + __u8 *md; + int ret; + + err = 1; + ret = bpf_dynptr_from_skb_meta(skb, 0, &meta); + if (ret) + return 1; + + err = 2; + md = bpf_dynptr_data(&meta, 0, sizeof(*md)); + if (md) + return 1; + + err = 0; + return 1; +} + SEC("tp/syscalls/sys_enter_nanosleep") int test_adjust(void *ctx) { @@ -567,6 +608,27 @@ int BPF_PROG(test_dynptr_skb_tp_btf, void *skb, void *location) return 1; } +SEC("tp_btf/kfree_skb") +int BPF_PROG(test_dynptr_skb_meta_tp_btf, void *skb, void *location) +{ + struct bpf_dynptr meta; + int ret; + + err = 1; + ret = bpf_dynptr_from_skb_meta(skb, 0, &meta); + if (ret) + return 1; + + /* Expect write failure. tp_btf skb is readonly. */ + err = 2; + ret = bpf_dynptr_write(&meta, 0, "x", 1, 0); + if (ret != -EINVAL) + return 1; + + err = 0; + return 1; +} + static inline int bpf_memcmp(const char *a, const char *b, u32 size) { int i; -- 2.43.0