* [PATCH bpf v2 0/2] Fix narrow loads from an offset outside of target field @ 2020-07-23 9:59 Jakub Sitnicki 2020-07-23 9:59 ` [PATCH bpf v2 1/2] bpf: Load zeros for narrow loads beyond " Jakub Sitnicki 2020-07-23 9:59 ` [PATCH bpf v2 2/2] selftests/bpf: Add test for narrow loads from context at an offset Jakub Sitnicki 0 siblings, 2 replies; 6+ messages in thread From: Jakub Sitnicki @ 2020-07-23 9:59 UTC (permalink / raw) To: bpf Cc: netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann, Yonghong Song This is a second attempt at fixing narrow loads from context fields backed by smaller-in-size target fields, when load offset is beyond the target field. Following Yonghong suggestion, verifier now emits an 'wX = 0' or 'rX = 0' instruction for loads from offsets outside of target field. Cc: Yonghong Song <yhs@fb.com> [v1] https://lore.kernel.org/bpf/20200710173123.427983-1-jakub@cloudflare.com/ Jakub Sitnicki (2): bpf: Load zeros for narrow loads beyond target field selftests/bpf: Add test for narrow loads from context at an offset kernel/bpf/verifier.c | 23 ++++- .../selftests/bpf/prog_tests/narrow_load.c | 84 +++++++++++++++++++ .../selftests/bpf/progs/test_narrow_load.c | 43 ++++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/narrow_load.c create mode 100644 tools/testing/selftests/bpf/progs/test_narrow_load.c -- 2.25.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf v2 1/2] bpf: Load zeros for narrow loads beyond target field 2020-07-23 9:59 [PATCH bpf v2 0/2] Fix narrow loads from an offset outside of target field Jakub Sitnicki @ 2020-07-23 9:59 ` Jakub Sitnicki 2020-07-23 21:56 ` Yonghong Song 2020-07-23 9:59 ` [PATCH bpf v2 2/2] selftests/bpf: Add test for narrow loads from context at an offset Jakub Sitnicki 1 sibling, 1 reply; 6+ messages in thread From: Jakub Sitnicki @ 2020-07-23 9:59 UTC (permalink / raw) To: bpf Cc: netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann, Yonghong Song For narrow loads from context that are: 1) as big in size as the target field, and 2) at an offset beyond the target field, the verifier does not emit the shift-and-mask instruction sequence following the target field load instruction, as it happens for narrow loads smaller in size than the target field width. This has an unexpected effect of loading the same data, no matter what the offset. While, arguably, the expected behavior is to load zeros for offsets that beyond the target field. For instance, 2-byte load from a 4-byte context field, backed by a 2-byte target field at an offset of 2 bytes results in: $ cat progs/test_narrow_load.c [...] SEC("sk_reuseport/narrow_load_half_word") int narrow_load_half_word(struct sk_reuseport_md *ctx) { __u16 *half; half = (__u16 *)&ctx->ip_protocol; if (half[0] != IPPROTO_UDP) return SK_DROP; if (half[1] != 0) return SK_DROP; return SK_PASS; } $ llvm-objdump -S --no-show-raw-insn ... [...] 0000000000000000 narrow_load_half_word: ; { 0: w0 = 0 ; if (half[0] != IPPROTO_UDP) 1: r2 = *(u16 *)(r1 + 24) 2: if w2 != 17 goto +4 <LBB1_3> ; if (half[1] != 0) 3: r1 = *(u16 *)(r1 + 26) 4: w0 = 1 5: if w1 == 0 goto +1 <LBB1_3> 6: w0 = 0 0000000000000038 LBB1_3: ; } 7: exit $ bpftool prog dump xlated ... int narrow_load_half_word(struct sk_reuseport_md * ctx): ; int narrow_load_half_word(struct sk_reuseport_md *ctx) 0: (b4) w0 = 0 ; if (half[0] != IPPROTO_UDP) 1: (79) r2 = *(u64 *)(r1 +8) 2: (69) r2 = *(u16 *)(r2 +924) ; if (half[0] != IPPROTO_UDP) 3: (56) if w2 != 0x11 goto pc+5 ; if (half[1] != 0) 4: (79) r1 = *(u64 *)(r1 +8) 5: (69) r1 = *(u16 *)(r1 +924) 6: (b4) w0 = 1 ; if (half[1] != 0) 7: (16) if w1 == 0x0 goto pc+1 8: (b4) w0 = 0 ; } 9: (95) exit In this case half[0] == half[1] == sk->sk_protocol, which is the target field for the ctx->ip_protocol. Fix it by emitting 'wX = 0' or 'rX = 0' instruction for all narrow loads from an offset that is beyond the target field. Going back to the example, with the fix in place, the upper half load from ctx->ip_protocol yields zero: int narrow_load_half_word(struct sk_reuseport_md * ctx): ; int narrow_load_half_word(struct sk_reuseport_md *ctx) 0: (b4) w0 = 0 ; if (half[0] != IPPROTO_UDP) 1: (79) r2 = *(u64 *)(r1 +8) 2: (69) r2 = *(u16 *)(r2 +924) ; if (half[0] != IPPROTO_UDP) 3: (56) if w2 != 0x11 goto pc+4 ; if (half[1] != 0) 4: (b4) w1 = 0 5: (b4) w0 = 1 ; if (half[1] != 0) 6: (16) if w1 == 0x0 goto pc+1 7: (b4) w0 = 0 ; } 8: (95) exit Fixes: f96da09473b5 ("bpf: simplify narrower ctx access") Suggested-by: Yonghong Song <yhs@fb.com> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> --- kernel/bpf/verifier.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 94cead5a43e5..0a9dbcdd6341 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9614,11 +9614,11 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, */ static int convert_ctx_accesses(struct bpf_verifier_env *env) { + u32 target_size, size_default, off, access_off; const struct bpf_verifier_ops *ops = env->ops; int i, cnt, size, ctx_field_size, delta = 0; const int insn_cnt = env->prog->len; struct bpf_insn insn_buf[16], *insn; - u32 target_size, size_default, off; struct bpf_prog *new_prog; enum bpf_access_type type; bool is_narrower_load; @@ -9760,7 +9760,26 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) return -EINVAL; } - if (is_narrower_load && size < target_size) { + /* When context field is wider than the target field, + * narrow load from an offset beyond the target field + * can be reduced to loading zero because there is + * nothing to load from memory. + */ + access_off = off & (size_default - 1); + if (is_narrower_load && access_off >= target_size) { + cnt = 0; + if (ctx_field_size <= 4) + insn_buf[cnt++] = BPF_MOV32_IMM(insn->dst_reg, 0); + else + insn_buf[cnt++] = BPF_MOV64_IMM(insn->dst_reg, 0); + } + /* Narrow load from an offset within the target field, + * smaller in size than the target field, needs + * shifting and masking because convert_ctx_access + * always emits full-size target field load. + */ + if (is_narrower_load && access_off < target_size && + size < target_size) { u8 shift = bpf_ctx_narrow_access_offset( off, size, size_default) * 8; if (ctx_field_size <= 4) { -- 2.25.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Load zeros for narrow loads beyond target field 2020-07-23 9:59 ` [PATCH bpf v2 1/2] bpf: Load zeros for narrow loads beyond " Jakub Sitnicki @ 2020-07-23 21:56 ` Yonghong Song 2020-07-27 14:53 ` Jakub Sitnicki 0 siblings, 1 reply; 6+ messages in thread From: Yonghong Song @ 2020-07-23 21:56 UTC (permalink / raw) To: Jakub Sitnicki, bpf Cc: netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann On 7/23/20 2:59 AM, Jakub Sitnicki wrote: > For narrow loads from context that are: > > 1) as big in size as the target field, and > 2) at an offset beyond the target field, > > the verifier does not emit the shift-and-mask instruction sequence > following the target field load instruction, as it happens for narrow loads > smaller in size than the target field width. > > This has an unexpected effect of loading the same data, no matter what the > offset. While, arguably, the expected behavior is to load zeros for offsets > that beyond the target field. > > For instance, 2-byte load from a 4-byte context field, backed by a 2-byte > target field at an offset of 2 bytes results in: > > $ cat progs/test_narrow_load.c > [...] > SEC("sk_reuseport/narrow_load_half_word") > int narrow_load_half_word(struct sk_reuseport_md *ctx) > { > __u16 *half; > > half = (__u16 *)&ctx->ip_protocol; > if (half[0] != IPPROTO_UDP) > return SK_DROP; > if (half[1] != 0) > return SK_DROP; > return SK_PASS; > } > > $ llvm-objdump -S --no-show-raw-insn ... > [...] > 0000000000000000 narrow_load_half_word: > ; { > 0: w0 = 0 > ; if (half[0] != IPPROTO_UDP) > 1: r2 = *(u16 *)(r1 + 24) > 2: if w2 != 17 goto +4 <LBB1_3> > ; if (half[1] != 0) > 3: r1 = *(u16 *)(r1 + 26) > 4: w0 = 1 > 5: if w1 == 0 goto +1 <LBB1_3> > 6: w0 = 0 > > 0000000000000038 LBB1_3: > ; } > 7: exit > > $ bpftool prog dump xlated ... > int narrow_load_half_word(struct sk_reuseport_md * ctx): > ; int narrow_load_half_word(struct sk_reuseport_md *ctx) > 0: (b4) w0 = 0 > ; if (half[0] != IPPROTO_UDP) > 1: (79) r2 = *(u64 *)(r1 +8) > 2: (69) r2 = *(u16 *)(r2 +924) > ; if (half[0] != IPPROTO_UDP) > 3: (56) if w2 != 0x11 goto pc+5 > ; if (half[1] != 0) > 4: (79) r1 = *(u64 *)(r1 +8) > 5: (69) r1 = *(u16 *)(r1 +924) > 6: (b4) w0 = 1 > ; if (half[1] != 0) > 7: (16) if w1 == 0x0 goto pc+1 > 8: (b4) w0 = 0 > ; } > 9: (95) exit > > In this case half[0] == half[1] == sk->sk_protocol, which is the target > field for the ctx->ip_protocol. > > Fix it by emitting 'wX = 0' or 'rX = 0' instruction for all narrow loads > from an offset that is beyond the target field. > > Going back to the example, with the fix in place, the upper half load from > ctx->ip_protocol yields zero: > > int narrow_load_half_word(struct sk_reuseport_md * ctx): > ; int narrow_load_half_word(struct sk_reuseport_md *ctx) > 0: (b4) w0 = 0 > ; if (half[0] != IPPROTO_UDP) > 1: (79) r2 = *(u64 *)(r1 +8) > 2: (69) r2 = *(u16 *)(r2 +924) > ; if (half[0] != IPPROTO_UDP) > 3: (56) if w2 != 0x11 goto pc+4 > ; if (half[1] != 0) > 4: (b4) w1 = 0 > 5: (b4) w0 = 1 > ; if (half[1] != 0) > 6: (16) if w1 == 0x0 goto pc+1 > 7: (b4) w0 = 0 > ; } > 8: (95) exit > > Fixes: f96da09473b5 ("bpf: simplify narrower ctx access") > Suggested-by: Yonghong Song <yhs@fb.com> > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> Thanks for the fix. The final code is much better now. Ack with some nits below. Acked-by: Yonghong Song <yhs@fb.com> > --- > kernel/bpf/verifier.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 94cead5a43e5..0a9dbcdd6341 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -9614,11 +9614,11 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, > */ > static int convert_ctx_accesses(struct bpf_verifier_env *env) > { > + u32 target_size, size_default, off, access_off; > const struct bpf_verifier_ops *ops = env->ops; > int i, cnt, size, ctx_field_size, delta = 0; > const int insn_cnt = env->prog->len; > struct bpf_insn insn_buf[16], *insn; > - u32 target_size, size_default, off; > struct bpf_prog *new_prog; > enum bpf_access_type type; > bool is_narrower_load; > @@ -9760,7 +9760,26 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) > return -EINVAL; > } > > - if (is_narrower_load && size < target_size) { > + /* When context field is wider than the target field, > + * narrow load from an offset beyond the target field > + * can be reduced to loading zero because there is > + * nothing to load from memory. Maybe it is worthwhile to mention that the below codegen undos what convert_ctx_access() just did. > + */ > + access_off = off & (size_default - 1); > + if (is_narrower_load && access_off >= target_size) { > + cnt = 0; > + if (ctx_field_size <= 4) > + insn_buf[cnt++] = BPF_MOV32_IMM(insn->dst_reg, 0); > + else > + insn_buf[cnt++] = BPF_MOV64_IMM(insn->dst_reg, 0); > + } > + /* Narrow load from an offset within the target field, > + * smaller in size than the target field, needs > + * shifting and masking because convert_ctx_access > + * always emits full-size target field load. > + */ > + if (is_narrower_load && access_off < target_size && > + size < target_size) { The code becomes a little bit complex here. I think it is worthwhile to have a static function to do codegen if is_narrower_load is true. The above two if statements are exclusive. It would be good to make it clear with "else if ...", and things will become easier if the narrower codegen is factored to a separate function. > u8 shift = bpf_ctx_narrow_access_offset( > off, size, size_default) * 8; > if (ctx_field_size <= 4) { > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 1/2] bpf: Load zeros for narrow loads beyond target field 2020-07-23 21:56 ` Yonghong Song @ 2020-07-27 14:53 ` Jakub Sitnicki 0 siblings, 0 replies; 6+ messages in thread From: Jakub Sitnicki @ 2020-07-27 14:53 UTC (permalink / raw) To: Yonghong Song Cc: bpf, netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann On Thu, Jul 23, 2020 at 11:56 PM CEST, Yonghong Song wrote: > On 7/23/20 2:59 AM, Jakub Sitnicki wrote: >> For narrow loads from context that are: >> >> 1) as big in size as the target field, and >> 2) at an offset beyond the target field, >> >> the verifier does not emit the shift-and-mask instruction sequence >> following the target field load instruction, as it happens for narrow loads >> smaller in size than the target field width. >> >> This has an unexpected effect of loading the same data, no matter what the >> offset. While, arguably, the expected behavior is to load zeros for offsets >> that beyond the target field. >> >> For instance, 2-byte load from a 4-byte context field, backed by a 2-byte >> target field at an offset of 2 bytes results in: >> >> $ cat progs/test_narrow_load.c >> [...] >> SEC("sk_reuseport/narrow_load_half_word") >> int narrow_load_half_word(struct sk_reuseport_md *ctx) >> { >> __u16 *half; >> >> half = (__u16 *)&ctx->ip_protocol; >> if (half[0] != IPPROTO_UDP) >> return SK_DROP; >> if (half[1] != 0) >> return SK_DROP; >> return SK_PASS; >> } >> >> $ llvm-objdump -S --no-show-raw-insn ... >> [...] >> 0000000000000000 narrow_load_half_word: >> ; { >> 0: w0 = 0 >> ; if (half[0] != IPPROTO_UDP) >> 1: r2 = *(u16 *)(r1 + 24) >> 2: if w2 != 17 goto +4 <LBB1_3> >> ; if (half[1] != 0) >> 3: r1 = *(u16 *)(r1 + 26) >> 4: w0 = 1 >> 5: if w1 == 0 goto +1 <LBB1_3> >> 6: w0 = 0 >> >> 0000000000000038 LBB1_3: >> ; } >> 7: exit >> >> $ bpftool prog dump xlated ... >> int narrow_load_half_word(struct sk_reuseport_md * ctx): >> ; int narrow_load_half_word(struct sk_reuseport_md *ctx) >> 0: (b4) w0 = 0 >> ; if (half[0] != IPPROTO_UDP) >> 1: (79) r2 = *(u64 *)(r1 +8) >> 2: (69) r2 = *(u16 *)(r2 +924) >> ; if (half[0] != IPPROTO_UDP) >> 3: (56) if w2 != 0x11 goto pc+5 >> ; if (half[1] != 0) >> 4: (79) r1 = *(u64 *)(r1 +8) >> 5: (69) r1 = *(u16 *)(r1 +924) >> 6: (b4) w0 = 1 >> ; if (half[1] != 0) >> 7: (16) if w1 == 0x0 goto pc+1 >> 8: (b4) w0 = 0 >> ; } >> 9: (95) exit >> >> In this case half[0] == half[1] == sk->sk_protocol, which is the target >> field for the ctx->ip_protocol. >> >> Fix it by emitting 'wX = 0' or 'rX = 0' instruction for all narrow loads >> from an offset that is beyond the target field. >> >> Going back to the example, with the fix in place, the upper half load from >> ctx->ip_protocol yields zero: >> >> int narrow_load_half_word(struct sk_reuseport_md * ctx): >> ; int narrow_load_half_word(struct sk_reuseport_md *ctx) >> 0: (b4) w0 = 0 >> ; if (half[0] != IPPROTO_UDP) >> 1: (79) r2 = *(u64 *)(r1 +8) >> 2: (69) r2 = *(u16 *)(r2 +924) >> ; if (half[0] != IPPROTO_UDP) >> 3: (56) if w2 != 0x11 goto pc+4 >> ; if (half[1] != 0) >> 4: (b4) w1 = 0 >> 5: (b4) w0 = 1 >> ; if (half[1] != 0) >> 6: (16) if w1 == 0x0 goto pc+1 >> 7: (b4) w0 = 0 >> ; } >> 8: (95) exit >> >> Fixes: f96da09473b5 ("bpf: simplify narrower ctx access") >> Suggested-by: Yonghong Song <yhs@fb.com> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > > Thanks for the fix. The final code is much better now. > Ack with some nits below. > > Acked-by: Yonghong Song <yhs@fb.com> > >> --- >> kernel/bpf/verifier.c | 23 +++++++++++++++++++++-- >> 1 file changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 94cead5a43e5..0a9dbcdd6341 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -9614,11 +9614,11 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, >> */ >> static int convert_ctx_accesses(struct bpf_verifier_env *env) >> { >> + u32 target_size, size_default, off, access_off; >> const struct bpf_verifier_ops *ops = env->ops; >> int i, cnt, size, ctx_field_size, delta = 0; >> const int insn_cnt = env->prog->len; >> struct bpf_insn insn_buf[16], *insn; >> - u32 target_size, size_default, off; >> struct bpf_prog *new_prog; >> enum bpf_access_type type; >> bool is_narrower_load; >> @@ -9760,7 +9760,26 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) >> return -EINVAL; >> } >> - if (is_narrower_load && size < target_size) { >> + /* When context field is wider than the target field, >> + * narrow load from an offset beyond the target field >> + * can be reduced to loading zero because there is >> + * nothing to load from memory. > > Maybe it is worthwhile to mention that the below codegen undos > what convert_ctx_access() just did. > >> + */ >> + access_off = off & (size_default - 1); >> + if (is_narrower_load && access_off >= target_size) { >> + cnt = 0; >> + if (ctx_field_size <= 4) >> + insn_buf[cnt++] = BPF_MOV32_IMM(insn->dst_reg, 0); >> + else >> + insn_buf[cnt++] = BPF_MOV64_IMM(insn->dst_reg, 0); >> + } >> + /* Narrow load from an offset within the target field, >> + * smaller in size than the target field, needs >> + * shifting and masking because convert_ctx_access >> + * always emits full-size target field load. >> + */ >> + if (is_narrower_load && access_off < target_size && >> + size < target_size) { > > The code becomes a little bit complex here. I think it is worthwhile > to have a static function to do codegen if is_narrower_load is true. > > The above two if statements are exclusive. It would be good to > make it clear with "else if ...", and things will become easier > if the narrower codegen is factored to a separate function. Thanks for comments. I will circle back to it in a bit. > >> u8 shift = bpf_ctx_narrow_access_offset( >> off, size, size_default) * 8; >> if (ctx_field_size <= 4) { >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf v2 2/2] selftests/bpf: Add test for narrow loads from context at an offset 2020-07-23 9:59 [PATCH bpf v2 0/2] Fix narrow loads from an offset outside of target field Jakub Sitnicki 2020-07-23 9:59 ` [PATCH bpf v2 1/2] bpf: Load zeros for narrow loads beyond " Jakub Sitnicki @ 2020-07-23 9:59 ` Jakub Sitnicki 2020-07-23 22:05 ` Yonghong Song 1 sibling, 1 reply; 6+ messages in thread From: Jakub Sitnicki @ 2020-07-23 9:59 UTC (permalink / raw) To: bpf; +Cc: netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann Check that narrow loads at various offsets from a context field backed by a target field that is smaller in size work as expected. That is target field value is loaded only when the offset is less than the target field size. While for offsets beyond the target field, the loaded value is zero. Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> --- .../selftests/bpf/prog_tests/narrow_load.c | 84 +++++++++++++++++++ .../selftests/bpf/progs/test_narrow_load.c | 43 ++++++++++ 2 files changed, 127 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/narrow_load.c create mode 100644 tools/testing/selftests/bpf/progs/test_narrow_load.c diff --git a/tools/testing/selftests/bpf/prog_tests/narrow_load.c b/tools/testing/selftests/bpf/prog_tests/narrow_load.c new file mode 100644 index 000000000000..6d79d722a66d --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/narrow_load.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause +// Copyright (c) 2020 Cloudflare + +#include "test_progs.h" +#include "test_narrow_load.skel.h" + +static int duration; + +void run_sk_reuseport_prog(struct bpf_program *reuseport_prog) +{ + static const struct timeval timeo = { .tv_sec = 3 }; + struct sockaddr_in addr = { + .sin_family = AF_INET, + .sin_port = 0, + .sin_addr.s_addr = htonl(INADDR_LOOPBACK), + }; + socklen_t len = sizeof(addr); + int err, fd, prog_fd; + const int one = 1; + char buf = 42; + ssize_t n; + + prog_fd = bpf_program__fd(reuseport_prog); + if (CHECK(prog_fd < 0, "bpf_program__fd", "errno %d\n", errno)) + return; + + fd = socket(AF_INET, SOCK_DGRAM, 0); + if (CHECK(fd < 0, "socket", "errno %d\n", errno)) + return; + + /* Setup timeouts */ + err = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo)); + if (CHECK(err, "setsockopt(SO_RCVTIMEO)", "errno %d\n", errno)) + goto out_close; + err = setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeo, sizeof(timeo)); + if (CHECK(err, "setsockopt(SO_RCVTIMEO)", "errno %d\n", errno)) + goto out_close; + + /* Setup reuseport prog */ + err = setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one, sizeof(one)); + if (CHECK(err, "setsockopt(SO_REUSEPORT)", "errno %d\n", errno)) + goto out_close; + err = setsockopt(fd, SOL_SOCKET, SO_ATTACH_REUSEPORT_EBPF, + &prog_fd, sizeof(prog_fd)); + if (CHECK(err, "setsockopt(SO_ATTACH_REUEPORT_EBPF)", + "errno %d\n", errno)) + goto out_close; + + err = bind(fd, (void *)&addr, len); + if (CHECK(err, "bind", "errno %d\n", errno)) + goto out_close; + err = getsockname(fd, (void *)&addr, &len); + if (CHECK(err, "getsockname", "errno %d\n", errno)) + goto out_close; + + /* Send a message to itself to trigger reuseport prog */ + n = sendto(fd, &buf, sizeof(buf), 0, (void *)&addr, len); + if (CHECK(n < 1, "sendto", "ret %ld errno %d\n", n, errno)) + goto out_close; + n = recv(fd, &buf, sizeof(buf), 0); + if (CHECK(n < 1, "recv", "ret %ld errno %d\n", n, errno)) + goto out_close; + + /* Pass, reuseport prog didn't drop the packet */ + +out_close: + close(fd); +} + +void test_narrow_load(void) +{ + struct test_narrow_load *skel; + + skel = test_narrow_load__open_and_load(); + if (CHECK(!skel, "skel open_and_load", "failed\n")) + return; + + if (test__start_subtest("narrow load byte")) + run_sk_reuseport_prog(skel->progs.narrow_load_byte); + if (test__start_subtest("narrow load half word")) + run_sk_reuseport_prog(skel->progs.narrow_load_half_word); + + test_narrow_load__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_narrow_load.c b/tools/testing/selftests/bpf/progs/test_narrow_load.c new file mode 100644 index 000000000000..57444720df16 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_narrow_load.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause +// Copyright (c) 2020 Cloudflare + +#include <stdint.h> + +#include <linux/bpf.h> +#include <linux/in.h> + +#include <bpf/bpf_helpers.h> + +char _license[] SEC("license") = "Dual BSD/GPL"; + +/* Check 1-byte load from 2-byte wide target field */ +SEC("sk_reuseport/narrow_load_byte") +int narrow_load_byte(struct sk_reuseport_md *ctx) +{ + __u8 *byte; + + byte = (__u8 *)&ctx->ip_protocol; + if (byte[0] != IPPROTO_UDP) + return SK_DROP; + if (byte[1] != 0) + return SK_DROP; + if (byte[2] != 0) + return SK_DROP; + if (byte[3] != 0) + return SK_DROP; + return SK_PASS; +} + +/* Check 2-byte load from 2-byte wide target field */ +SEC("sk_reuseport/narrow_load_half_word") +int narrow_load_half_word(struct sk_reuseport_md *ctx) +{ + __u16 *half; + + half = (__u16 *)&ctx->ip_protocol; + if (half[0] != IPPROTO_UDP) + return SK_DROP; + if (half[1] != 0) + return SK_DROP; + return SK_PASS; +} -- 2.25.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf v2 2/2] selftests/bpf: Add test for narrow loads from context at an offset 2020-07-23 9:59 ` [PATCH bpf v2 2/2] selftests/bpf: Add test for narrow loads from context at an offset Jakub Sitnicki @ 2020-07-23 22:05 ` Yonghong Song 0 siblings, 0 replies; 6+ messages in thread From: Yonghong Song @ 2020-07-23 22:05 UTC (permalink / raw) To: Jakub Sitnicki, bpf Cc: netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann On 7/23/20 2:59 AM, Jakub Sitnicki wrote: > Check that narrow loads at various offsets from a context field backed by a > target field that is smaller in size work as expected. That is target field > value is loaded only when the offset is less than the target field size. > While for offsets beyond the target field, the loaded value is zero. > > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> Ack with minor nit below. Acked-by: Yonghong Song <yhs@fb.com> > --- > .../selftests/bpf/prog_tests/narrow_load.c | 84 +++++++++++++++++++ > .../selftests/bpf/progs/test_narrow_load.c | 43 ++++++++++ > 2 files changed, 127 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/narrow_load.c > create mode 100644 tools/testing/selftests/bpf/progs/test_narrow_load.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/narrow_load.c b/tools/testing/selftests/bpf/prog_tests/narrow_load.c > new file mode 100644 > index 000000000000..6d79d722a66d > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/narrow_load.c > @@ -0,0 +1,84 @@ > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause > +// Copyright (c) 2020 Cloudflare > + > +#include "test_progs.h" > +#include "test_narrow_load.skel.h" > + > +static int duration; > + > +void run_sk_reuseport_prog(struct bpf_program *reuseport_prog) static function? > +{ > + static const struct timeval timeo = { .tv_sec = 3 }; > + struct sockaddr_in addr = { > + .sin_family = AF_INET, > + .sin_port = 0, > + .sin_addr.s_addr = htonl(INADDR_LOOPBACK), > + }; > + socklen_t len = sizeof(addr); > + int err, fd, prog_fd; > + const int one = 1; > + char buf = 42; > + ssize_t n; > + [...] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-27 14:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-23 9:59 [PATCH bpf v2 0/2] Fix narrow loads from an offset outside of target field Jakub Sitnicki 2020-07-23 9:59 ` [PATCH bpf v2 1/2] bpf: Load zeros for narrow loads beyond " Jakub Sitnicki 2020-07-23 21:56 ` Yonghong Song 2020-07-27 14:53 ` Jakub Sitnicki 2020-07-23 9:59 ` [PATCH bpf v2 2/2] selftests/bpf: Add test for narrow loads from context at an offset Jakub Sitnicki 2020-07-23 22:05 ` Yonghong Song
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.