| Message ID | 20231212131031.3088661-1-menglong8.dong@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpf: support to trace BPF_JNE | expand |
On Tue, Dec 12, 2023 at 5:15 AM Menglong Dong <menglong8.dong@gmail.com> wrote: > > For now, the reg bounds is not handled for BPF_JNE case, which can cause > the failure of following case: > > /* The type of "a" is u16 */ > if (a > 0 && a < 100) { > /* the range of the register for a is [0, 99], not [1, 99], > * and will cause the following error: > * > * invalid zero-sized read > * > * as a can be 0. > */ > bpf_skb_store_bytes(skb, xx, xx, a, 0); > } > > In the code above, "a > 0" will be compiled to "jmp xxx if a == 0". In the > TRUE branch, the dst_reg will be marked as known to 0. However, in the > fallthrough(FALSE) branch, the dst_reg will not be handled, which makes > the [min, max] for a is [0, 99], not [1, 99]. > > In the 1st patch, we reduce the range of the dst reg if the src reg is a > const and is exactly the edge of the dst reg For BPF_JNE. > > In the 2nd patch, we just activate the test case for this logic in > range_cond(), which is committed by Andrii in the > commit 8863238993e2 ("selftests/bpf: BPF register range bounds tester"). > > Changes since v1: > - simplify the code in the 1st patch > - introduce the 2nd patch for the testing > > Menglong Dong (2): > bpf: make the verifier trace the "not qeual" for regs > selftests/bpf: activate the OP_NE login in range_cond() > > kernel/bpf/verifier.c | 29 ++++++++++++++++++- > .../selftests/bpf/prog_tests/reg_bounds.c | 7 +---- > 2 files changed, 29 insertions(+), 7 deletions(-) > > -- > 2.39.2 > +1 to all the feedback from Eduard. Besides that, please target bpf-next tree (so, [PATH bpf-next] for subject prefix), thanks! Also, instead of "verifier traces", I think "verifier tracks" is less confusing wording. Tracing within the BPF ecosystem is usually used for a completely different meaning. Oh, and just to keep feedback in one place. In patch #2 you have a typo in the subject "not qeual" -> "not equal".
On Wed, Dec 13, 2023 at 8:00 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Dec 12, 2023 at 5:15 AM Menglong Dong <menglong8.dong@gmail.com> wrote: > > > > For now, the reg bounds is not handled for BPF_JNE case, which can cause > > the failure of following case: > > > > /* The type of "a" is u16 */ > > if (a > 0 && a < 100) { > > /* the range of the register for a is [0, 99], not [1, 99], > > * and will cause the following error: > > * > > * invalid zero-sized read > > * > > * as a can be 0. > > */ > > bpf_skb_store_bytes(skb, xx, xx, a, 0); > > } > > > > In the code above, "a > 0" will be compiled to "jmp xxx if a == 0". In the > > TRUE branch, the dst_reg will be marked as known to 0. However, in the > > fallthrough(FALSE) branch, the dst_reg will not be handled, which makes > > the [min, max] for a is [0, 99], not [1, 99]. > > > > In the 1st patch, we reduce the range of the dst reg if the src reg is a > > const and is exactly the edge of the dst reg For BPF_JNE. > > > > In the 2nd patch, we just activate the test case for this logic in > > range_cond(), which is committed by Andrii in the > > commit 8863238993e2 ("selftests/bpf: BPF register range bounds tester"). > > > > Changes since v1: > > - simplify the code in the 1st patch > > - introduce the 2nd patch for the testing > > > > Menglong Dong (2): > > bpf: make the verifier trace the "not qeual" for regs > > selftests/bpf: activate the OP_NE login in range_cond() > > > > kernel/bpf/verifier.c | 29 ++++++++++++++++++- > > .../selftests/bpf/prog_tests/reg_bounds.c | 7 +---- > > 2 files changed, 29 insertions(+), 7 deletions(-) > > > > -- > > 2.39.2 > > > > +1 to all the feedback from Eduard. Besides that, please target > bpf-next tree (so, [PATH bpf-next] for subject prefix), thanks! > Opps, sorry that I offered a wrong tag......:/ > Also, instead of "verifier traces", I think "verifier tracks" is less > confusing wording. Tracing within the BPF ecosystem is usually used > for a completely different meaning. > Yeah, sounds better. > Oh, and just to keep feedback in one place. In patch #2 you have a > typo in the subject "not qeual" -> "not equal". Ok, I'll fix it in the next version. Thanks! Menglong Dong
For now, the reg bounds is not handled for BPF_JNE case, which can cause the failure of following case: /* The type of "a" is u16 */ if (a > 0 && a < 100) { /* the range of the register for a is [0, 99], not [1, 99], * and will cause the following error: * * invalid zero-sized read * * as a can be 0. */ bpf_skb_store_bytes(skb, xx, xx, a, 0); } In the code above, "a > 0" will be compiled to "jmp xxx if a == 0". In the TRUE branch, the dst_reg will be marked as known to 0. However, in the fallthrough(FALSE) branch, the dst_reg will not be handled, which makes the [min, max] for a is [0, 99], not [1, 99]. In the 1st patch, we reduce the range of the dst reg if the src reg is a const and is exactly the edge of the dst reg For BPF_JNE. In the 2nd patch, we just activate the test case for this logic in range_cond(), which is committed by Andrii in the commit 8863238993e2 ("selftests/bpf: BPF register range bounds tester"). Changes since v1: - simplify the code in the 1st patch - introduce the 2nd patch for the testing Menglong Dong (2): bpf: make the verifier trace the "not qeual" for regs selftests/bpf: activate the OP_NE login in range_cond() kernel/bpf/verifier.c | 29 ++++++++++++++++++- .../selftests/bpf/prog_tests/reg_bounds.c | 7 +---- 2 files changed, 29 insertions(+), 7 deletions(-)