On Mon, Jul 25, 2022 at 08:31:01AM +0100, Alan Maguire wrote:
> add tests that verify bpf_snprintf_btf() behaviour with strings that
>
> - exactly fit the buffer (string size + null terminator == buffer_size)
> - overrun the buffer (string size + null terminator == buffer size + 1)
> - overrun the buffer (string size + null terminator == buffer size + 2)
>
> These tests require [1] ("bpf: btf: Fix vsnprintf return value check")
>
> ...which has not landed yet.
patch looks good, but I have the test passing even without [1],
it should fail, right?
#151 snprintf_btf:OK
jirka
>
> [1] https://lore.kernel.org/bpf/20220711211317.GA1143610@laptop/
>
> Suggested-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> ---
> .../selftests/bpf/progs/netif_receive_skb.c | 41 ++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> index 1d8918d..9fc48e4 100644
> --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c
> @@ -49,7 +49,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> }
>
> #if __has_builtin(__builtin_btf_type_id)
> -#define TEST_BTF(_str, _type, _flags, _expected, ...) \
> +#define TEST_BTF_SIZE(_str, _size, _type, _flags, _expected, ...) \
> do { \
> static const char _expectedval[EXPECTED_STRSIZE] = \
> _expected; \
> @@ -69,10 +69,13 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> ret = -EINVAL; \
> break; \
> } \
> - ret = bpf_snprintf_btf(_str, STRSIZE, \
> + ret = bpf_snprintf_btf(_str, _size, \
> &_ptr, sizeof(_ptr), _hflags); \
> - if (ret) \
> + if (ret < 0) { \
> + bpf_printk("bpf_snprintf_btf_failed (%s): %d\n",\
> + _str, _expectedval, ret); \
> break; \
> + } \
> _cmp = __strncmp(_str, _expectedval, EXPECTED_STRSIZE); \
> if (_cmp != 0) { \
> bpf_printk("(%d) got %s", _cmp, _str); \
> @@ -82,6 +85,10 @@ static int __strncmp(const void *m1, const void *m2, size_t len)
> break; \
> } \
> } while (0)
> +
> +#define TEST_BTF(_str, _type, _flags, _expected, ...) \
> + TEST_BTF_SIZE(_str, STRSIZE, _type, _flags, _expected, \
> + __VA_ARGS__)
> #endif
>
> /* Use where expected data string matches its stringified declaration */
> @@ -98,7 +105,9 @@ int BPF_PROG(trace_netif_receive_skb, struct sk_buff *skb)
> static __u64 flags[] = { 0, BTF_F_COMPACT, BTF_F_ZERO, BTF_F_PTR_RAW,
> BTF_F_NONAME, BTF_F_COMPACT | BTF_F_ZERO |
> BTF_F_PTR_RAW | BTF_F_NONAME };
> + static char _short_str[2] = {};
> static struct btf_ptr p = { };
> + char *short_str = _short_str;
> __u32 key = 0;
> int i, __ret;
> char *str;
> @@ -141,6 +150,32 @@ int BPF_PROG(trace_netif_receive_skb, struct sk_buff *skb)
> TEST_BTF_C(str, int, 0, -4567);
> TEST_BTF(str, int, BTF_F_NONAME, "-4567", -4567);
>
> + /* overflow tests; first string + terminator fits, others do not. */
> + TEST_BTF_SIZE(short_str, sizeof(_short_str), int, BTF_F_NONAME, "1", 1);
> + if (ret != 1) {
> + bpf_printk("bpf_snprintf_btf() should return 1 for '%s'/2-byte array",
> + short_str);
> + ret = -ERANGE;
> + }
> + /* not enough space to write "10", write "1", return 2 for number of bytes we
> + * should have written.
> + */
> + TEST_BTF_SIZE(short_str, sizeof(_short_str), int, BTF_F_NONAME, "1", 10);
> + if (ret != 2) {
> + bpf_printk("bpf_snprintf_btf() should return 2 for '%s'/2-byte array",
> + short_str);
> + ret = -ERANGE;
> + }
> + /* not enough space to write "100", write "1", return 3 for number of bytes we
> + * should have written.
> + */
> + TEST_BTF_SIZE(short_str, sizeof(_short_str), int, BTF_F_NONAME, "1", 100);
> + if (ret != 3) {
> + bpf_printk("bpf_snprintf_btf() should return 3 for '%s'/3-byte array",
> + short_str);
> + ret = -ERANGE;
> + }
> +
> /* simple char */
> TEST_BTF_C(str, char, 0, 100);
> TEST_BTF(str, char, BTF_F_NONAME, "100", 100);
> --
> 1.8.3.1
>
