[v2] bpf: enhance validation of pointer formatting

[PATCH bpf v2 2/2] selftests/bpf: Add test cases for various pointer specifiers

Posted by Ilya Shchipletsov 3 weeks, 6 days ago

Extend snprintf negative tests to cover pointer specifiers to prevent possible
invalid handling of %p% from happening again.

 ./test_progs -t snprintf
 #302/1   snprintf/snprintf_positive:OK
 #302/2   snprintf/snprintf_negative:OK
 #302     snprintf:OK
 #303     snprintf_btf:OK
 Summary: 2/2 PASSED, 0 SKIPPED, 0 FAILED

Co-developed-by: Nikita Marushkin <hfggklm@gmail.com>
Signed-off-by: Nikita Marushkin <hfggklm@gmail.com>
Signed-off-by: Ilya Shchipletsov <rabbelkin@mail.ru>
---
 tools/testing/selftests/bpf/prog_tests/snprintf.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf.c b/tools/testing/selftests/bpf/prog_tests/snprintf.c
index 4be6fdb78c6a..b5b6371e09bb 100644
--- a/tools/testing/selftests/bpf/prog_tests/snprintf.c
+++ b/tools/testing/selftests/bpf/prog_tests/snprintf.c
@@ -116,6 +116,21 @@ static void test_snprintf_negative(void)
 	ASSERT_ERR(load_single_snprintf("%llc"), "invalid specifier 7");
 	ASSERT_ERR(load_single_snprintf("\x80"), "non ascii character");
 	ASSERT_ERR(load_single_snprintf("\x1"), "non printable character");
+
+	ASSERT_OK(load_single_snprintf("valid %p"), "valid usage");
+
+	ASSERT_ERR(load_single_snprintf("%p%"), "too many specifiers 1");
+	ASSERT_ERR(load_single_snprintf("%pK%"), "too many specifiers 2");
+	ASSERT_ERR(load_single_snprintf("%px%"), "too many specifiers 3");
+	ASSERT_ERR(load_single_snprintf("%ps%"), "too many specifiers 4");
+	ASSERT_ERR(load_single_snprintf("%pS%"), "too many specifiers 5");
+	ASSERT_ERR(load_single_snprintf("%pB%"), "too many specifiers 6");
+	ASSERT_ERR(load_single_snprintf("%pi4%"), "too many specifiers 7");
+	ASSERT_ERR(load_single_snprintf("%pI4%"), "too many specifiers 8");
+	ASSERT_ERR(load_single_snprintf("%pi6%"), "too many specifiers 9");
+	ASSERT_ERR(load_single_snprintf("%pI6%"), "too many specifiers 10");
+	ASSERT_ERR(load_single_snprintf("%pks%"), "too many specifiers 11");
+	ASSERT_ERR(load_single_snprintf("%pus%"), "too many specifiers 12");
 }
 
 void test_snprintf(void)
-- 
2.43.0

Re: [PATCH bpf v2 2/2] selftests/bpf: Add test cases for various pointer specifiers

Posted by Yonghong Song 3 weeks, 6 days ago

On 10/28/24 12:53 PM, Ilya Shchipletsov wrote:
> Extend snprintf negative tests to cover pointer specifiers to prevent possible
> invalid handling of %p% from happening again.
>
>   ./test_progs -t snprintf
>   #302/1   snprintf/snprintf_positive:OK
>   #302/2   snprintf/snprintf_negative:OK
>   #302     snprintf:OK
>   #303     snprintf_btf:OK
>   Summary: 2/2 PASSED, 0 SKIPPED, 0 FAILED
>
> Co-developed-by: Nikita Marushkin <hfggklm@gmail.com>
> Signed-off-by: Nikita Marushkin <hfggklm@gmail.com>
> Signed-off-by: Ilya Shchipletsov <rabbelkin@mail.ru>

Acked-by: Yonghong Song <yonghong.song@linux.dev>

Re: [PATCH bpf v2 2/2] selftests/bpf: Add test cases for various pointer specifiers

Posted by Florent Revest 3 weeks, 3 days ago

Thank you!

Acked-by: Florent Revest <revest@chromium.org>

On Tue, Oct 29, 2024 at 7:19 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 10/28/24 12:53 PM, Ilya Shchipletsov wrote:
> > Extend snprintf negative tests to cover pointer specifiers to prevent possible
> > invalid handling of %p% from happening again.
> >
> >   ./test_progs -t snprintf
> >   #302/1   snprintf/snprintf_positive:OK
> >   #302/2   snprintf/snprintf_negative:OK
> >   #302     snprintf:OK
> >   #303     snprintf_btf:OK
> >   Summary: 2/2 PASSED, 0 SKIPPED, 0 FAILED
> >
> > Co-developed-by: Nikita Marushkin <hfggklm@gmail.com>
> > Signed-off-by: Nikita Marushkin <hfggklm@gmail.com>
> > Signed-off-by: Ilya Shchipletsov <rabbelkin@mail.ru>
>
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>