On Mon, Apr 19, 2021 at 9:34 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Mon, Apr 19, 2021 at 8:52 AM Florent Revest <revest@xxxxxxxxxxxx> 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@xxxxxxxxxx/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@xxxxxxxxxxxx/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@xxxxxxxxxx> Thank you for the all the fast and in-depth reviews Andrii! :)
