* [PATCH bpf-next] selftests/bpf: augment snprintf_btf tests with string overflow tests
@ 2022-07-25 7:31 Alan Maguire
2022-07-26 12:39 ` Jiri Olsa
0 siblings, 1 reply; 3+ messages in thread
From: Alan Maguire @ 2022-07-25 7:31 UTC (permalink / raw)
To: andrii, mykolal, ftokarev, jolsa
Cc: ast, daniel, martin.lau, song, yhs, john.fastabend, kpsingh, sdf,
haoluo, bpf, Alan Maguire
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.
[1] https://lore.kernel.org/bpf/20220711211317.GA1143610@laptop/
Suggested-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
.../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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: augment snprintf_btf tests with string overflow tests
2022-07-25 7:31 [PATCH bpf-next] selftests/bpf: augment snprintf_btf tests with string overflow tests Alan Maguire
@ 2022-07-26 12:39 ` Jiri Olsa
2022-07-29 16:57 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Jiri Olsa @ 2022-07-26 12:39 UTC (permalink / raw)
To: Alan Maguire
Cc: andrii, mykolal, ftokarev, jolsa, ast, daniel, martin.lau, song,
yhs, john.fastabend, kpsingh, sdf, haoluo, bpf
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@redhat.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> .../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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: augment snprintf_btf tests with string overflow tests
2022-07-26 12:39 ` Jiri Olsa
@ 2022-07-29 16:57 ` Andrii Nakryiko
0 siblings, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2022-07-29 16:57 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alan Maguire, Andrii Nakryiko, Mykola Lysenko, Fedor Tokarev,
Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
Song Liu, Yonghong Song, john fastabend, KP Singh,
Stanislav Fomichev, Hao Luo, bpf
On Tue, Jul 26, 2022 at 5:40 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> 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
>
The way that test is structured it's essentially impossible to
communicate a test failure back, as each subsequent test case
overrides common ret variable, if I understand it correctly.
Alan, can you please update the test to store results for each
individual test case separately, e.g., in an array?
Meanwhile I've applied that trivial fix from Fedor anyway.
> jirka
>
> >
> > [1] https://lore.kernel.org/bpf/20220711211317.GA1143610@laptop/
> >
> > Suggested-by: Jiri Olsa <jolsa@redhat.com>
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> > .../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)
> > }
> >
[...]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-29 16:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-25 7:31 [PATCH bpf-next] selftests/bpf: augment snprintf_btf tests with string overflow tests Alan Maguire
2022-07-26 12:39 ` Jiri Olsa
2022-07-29 16:57 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).