| Message ID | 20210419155243.1632274-1-revest@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add a snprintf eBPF helper | expand |
On Mon, Apr 19, 2021 at 8:52 AM Florent Revest <revest@chromium.org> wrote: > > We have a usecase where we want to audit symbol names (if available) in > callback registration hooks. (ex: fentry/nf_register_net_hook) > > A few months back, I proposed a bpf_kallsyms_lookup series but it was > decided in the reviews that a more generic helper, bpf_snprintf, would > be more useful. > > This series implements the helper according to the feedback received in > https://lore.kernel.org/bpf/20201126165748.1748417-1-revest@google.com/T/#u > > - A new arg type guarantees the NULL-termination of string arguments and > lets us pass format strings in only one arg > - A new helper is implemented using that guarantee. Because the format > string is known at verification time, the format string validation is > done by the verifier > - To implement a series of tests for bpf_snprintf, the logic for > marshalling variadic args in a fixed-size array is reworked as per: > https://lore.kernel.org/bpf/20210310015455.1095207-1-revest@chromium.org/T/#u > > --- > Changes in v5: > - Fixed the bpf_printf_buf_used counter logic in try_get_fmt_tmp_buf > - Added a couple of extra incorrect specifiers tests > - Call test_snprintf_single__destroy unconditionally > - Fixed a C++-style comment > > --- > Changes in v4: > - Moved bpf_snprintf, bpf_printf_prepare and bpf_printf_cleanup to > kernel/bpf/helpers.c so that they get built without CONFIG_BPF_EVENTS > - Added negative test cases (various invalid format strings) > - Renamed put_fmt_tmp_buf() as bpf_printf_cleanup() > - Fixed a mistake that caused temporary buffers to be unconditionally > freed in bpf_printf_prepare > - Fixed a mistake that caused missing 0 character to be ignored > - Fixed a warning about integer to pointer conversion > - Misc cleanups > > --- > Changes in v3: > - Simplified temporary buffer acquisition with try_get_fmt_tmp_buf() > - Made zero-termination check more consistent > - Allowed NULL output_buffer > - Simplified the BPF_CAST_FMT_ARG macro > - Three new test cases: number padding, simple string with no arg and > string length extraction only with a NULL output buffer > - Clarified helper's description for edge cases (eg: str_size == 0) > - Lots of cosmetic changes > > --- > Changes in v2: > - Extracted the format validation/argument sanitization in a generic way > for all printf-like helpers. > - bpf_snprintf's str_size can now be 0 > - bpf_snprintf is now exposed to all BPF program types > - We now preempt_disable when using a per-cpu temporary buffer > - Addressed a few cosmetic changes > > Florent Revest (6): > bpf: Factorize bpf_trace_printk and bpf_seq_printf > bpf: Add a ARG_PTR_TO_CONST_STR argument type > bpf: Add a bpf_snprintf helper > libbpf: Initialize the bpf_seq_printf parameters array field by field > libbpf: Introduce a BPF_SNPRINTF helper macro > selftests/bpf: Add a series of tests for bpf_snprintf > > include/linux/bpf.h | 22 ++ > include/uapi/linux/bpf.h | 28 ++ > kernel/bpf/helpers.c | 306 ++++++++++++++ > kernel/bpf/verifier.c | 82 ++++ > kernel/trace/bpf_trace.c | 373 ++---------------- > tools/include/uapi/linux/bpf.h | 28 ++ > tools/lib/bpf/bpf_tracing.h | 58 ++- > .../selftests/bpf/prog_tests/snprintf.c | 125 ++++++ > .../selftests/bpf/progs/test_snprintf.c | 73 ++++ > .../bpf/progs/test_snprintf_single.c | 20 + > 10 files changed, 770 insertions(+), 345 deletions(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf.c > create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf.c > create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf_single.c > > -- > 2.31.1.368.gbe11c130af-goog > Looks great, thank you! For the series: Acked-by: Andrii Nakryiko <andrii@kernel.org>
On Mon, Apr 19, 2021 at 9:34 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Apr 19, 2021 at 8:52 AM Florent Revest <revest@chromium.org> wrote: > > > > We have a usecase where we want to audit symbol names (if available) in > > callback registration hooks. (ex: fentry/nf_register_net_hook) > > > > A few months back, I proposed a bpf_kallsyms_lookup series but it was > > decided in the reviews that a more generic helper, bpf_snprintf, would > > be more useful. > > > > This series implements the helper according to the feedback received in > > https://lore.kernel.org/bpf/20201126165748.1748417-1-revest@google.com/T/#u > > > > - A new arg type guarantees the NULL-termination of string arguments and > > lets us pass format strings in only one arg > > - A new helper is implemented using that guarantee. Because the format > > string is known at verification time, the format string validation is > > done by the verifier > > - To implement a series of tests for bpf_snprintf, the logic for > > marshalling variadic args in a fixed-size array is reworked as per: > > https://lore.kernel.org/bpf/20210310015455.1095207-1-revest@chromium.org/T/#u > > > > --- > > Changes in v5: > > - Fixed the bpf_printf_buf_used counter logic in try_get_fmt_tmp_buf > > - Added a couple of extra incorrect specifiers tests > > - Call test_snprintf_single__destroy unconditionally > > - Fixed a C++-style comment > > > > --- > > Changes in v4: > > - Moved bpf_snprintf, bpf_printf_prepare and bpf_printf_cleanup to > > kernel/bpf/helpers.c so that they get built without CONFIG_BPF_EVENTS > > - Added negative test cases (various invalid format strings) > > - Renamed put_fmt_tmp_buf() as bpf_printf_cleanup() > > - Fixed a mistake that caused temporary buffers to be unconditionally > > freed in bpf_printf_prepare > > - Fixed a mistake that caused missing 0 character to be ignored > > - Fixed a warning about integer to pointer conversion > > - Misc cleanups > > > > --- > > Changes in v3: > > - Simplified temporary buffer acquisition with try_get_fmt_tmp_buf() > > - Made zero-termination check more consistent > > - Allowed NULL output_buffer > > - Simplified the BPF_CAST_FMT_ARG macro > > - Three new test cases: number padding, simple string with no arg and > > string length extraction only with a NULL output buffer > > - Clarified helper's description for edge cases (eg: str_size == 0) > > - Lots of cosmetic changes > > > > --- > > Changes in v2: > > - Extracted the format validation/argument sanitization in a generic way > > for all printf-like helpers. > > - bpf_snprintf's str_size can now be 0 > > - bpf_snprintf is now exposed to all BPF program types > > - We now preempt_disable when using a per-cpu temporary buffer > > - Addressed a few cosmetic changes > > > > Florent Revest (6): > > bpf: Factorize bpf_trace_printk and bpf_seq_printf > > bpf: Add a ARG_PTR_TO_CONST_STR argument type > > bpf: Add a bpf_snprintf helper > > libbpf: Initialize the bpf_seq_printf parameters array field by field > > libbpf: Introduce a BPF_SNPRINTF helper macro > > selftests/bpf: Add a series of tests for bpf_snprintf > > > > include/linux/bpf.h | 22 ++ > > include/uapi/linux/bpf.h | 28 ++ > > kernel/bpf/helpers.c | 306 ++++++++++++++ > > kernel/bpf/verifier.c | 82 ++++ > > kernel/trace/bpf_trace.c | 373 ++---------------- > > tools/include/uapi/linux/bpf.h | 28 ++ > > tools/lib/bpf/bpf_tracing.h | 58 ++- > > .../selftests/bpf/prog_tests/snprintf.c | 125 ++++++ > > .../selftests/bpf/progs/test_snprintf.c | 73 ++++ > > .../bpf/progs/test_snprintf_single.c | 20 + > > 10 files changed, 770 insertions(+), 345 deletions(-) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf.c > > create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf.c > > create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf_single.c > > > > -- > > 2.31.1.368.gbe11c130af-goog > > > > Looks great, thank you! > > For the series: > > Acked-by: Andrii Nakryiko <andrii@kernel.org> Thank you for the all the fast and in-depth reviews Andrii! :)