[PATCH bpf 1/2] bpf: Reject %p% format string in bprintf-like helpers

[PATCH bpf 1/2] bpf: Reject %p% format string in bprintf-like helpers

[Paul Chaignon]

    static const char fmt[] = "%p%";
    bpf_trace_printk(fmt, sizeof(fmt));

The above BPF program isn't rejected and causes a kernel warning at
runtime:

    Please remove unsupported %\x00 in format string
    WARNING: CPU: 1 PID: 7244 at lib/vsprintf.c:2680 format_decode+0x49c/0x5d0

This happens because bpf_bprintf_prepare skips over the second %,
detected as punctuation, while processing %p. This patch fixes it by
not skipping over punctuation. %\x00 is then processed in the next
iteration and rejected.

Reported-by: syzbot+e2c932aec5c8a6e1d31c@syzkaller.appspotmail.com
Fixes: 48cac3f4a96d ("bpf: Implement formatted output helpers with bstr_printf")
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 kernel/bpf/helpers.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b71e428ad936..ad6df48b540c 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -884,6 +884,13 @@ int bpf_bprintf_prepare(char *fmt, u32 fmt_size, const u64 *raw_args,
 		if (fmt[i] == 'p') {
 			sizeof_cur_arg = sizeof(long);
 
+			if (fmt[i + 1] == 0 || isspace(fmt[i + 1]) ||
+			    ispunct(fmt[i + 1])) {
+				if (tmp_buf)
+					cur_arg = raw_args[num_spec];
+				goto nocopy_fmt;
+			}
+
 			if ((fmt[i + 1] == 'k' || fmt[i + 1] == 'u') &&
 			    fmt[i + 2] == 's') {
 				fmt_ptype = fmt[i + 1];
@@ -891,11 +898,9 @@ int bpf_bprintf_prepare(char *fmt, u32 fmt_size, const u64 *raw_args,
 				goto fmt_str;
 			}
 
-			if (fmt[i + 1] == 0 || isspace(fmt[i + 1]) ||
-			    ispunct(fmt[i + 1]) || fmt[i + 1] == 'K' ||
+			if (fmt[i + 1] == 'K' ||
 			    fmt[i + 1] == 'x' || fmt[i + 1] == 's' ||
 			    fmt[i + 1] == 'S') {
-				/* just kernel pointers */
 				if (tmp_buf)
 					cur_arg = raw_args[num_spec];
 				i++;
-- 
2.43.0

[PATCH bpf 2/2] selftests/bpf: Add negative test cases for snprintf

[Paul Chaignon]

This patch adds a couple negative test cases with a trailing % at the
end of the format string.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/snprintf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf.c b/tools/testing/selftests/bpf/prog_tests/snprintf.c
index 4be6fdb78c6a..594441acb707 100644
--- a/tools/testing/selftests/bpf/prog_tests/snprintf.c
+++ b/tools/testing/selftests/bpf/prog_tests/snprintf.c
@@ -116,6 +116,8 @@ 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_ERR(load_single_snprintf("%p%"), "invalid specifier 8");
+	ASSERT_ERR(load_single_snprintf("%s%"), "invalid specifier 9");
 }
 
 void test_snprintf(void)
-- 
2.43.0

Re: [PATCH bpf 1/2] bpf: Reject %p% format string in bprintf-like helpers

[Yonghong Song]

On 6/26/25 9:06 AM, Paul Chaignon wrote:
>      static const char fmt[] = "%p%";
>      bpf_trace_printk(fmt, sizeof(fmt));
>
> The above BPF program isn't rejected and causes a kernel warning at
> runtime:
>
>      Please remove unsupported %\x00 in format string
>      WARNING: CPU: 1 PID: 7244 at lib/vsprintf.c:2680 format_decode+0x49c/0x5d0
>
> This happens because bpf_bprintf_prepare skips over the second %,
> detected as punctuation, while processing %p. This patch fixes it by
> not skipping over punctuation. %\x00 is then processed in the next
> iteration and rejected.
>
> Reported-by: syzbot+e2c932aec5c8a6e1d31c@syzkaller.appspotmail.com
> Fixes: 48cac3f4a96d ("bpf: Implement formatted output helpers with bstr_printf")
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>

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

Re: [PATCH bpf 2/2] selftests/bpf: Add negative test cases for snprintf

[Yonghong Song]

On 6/26/25 9:07 AM, Paul Chaignon wrote:
> This patch adds a couple negative test cases with a trailing % at the
> end of the format string.
>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>

LGTM with a nit below.

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

> ---
>   tools/testing/selftests/bpf/prog_tests/snprintf.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf.c b/tools/testing/selftests/bpf/prog_tests/snprintf.c
> index 4be6fdb78c6a..594441acb707 100644
> --- a/tools/testing/selftests/bpf/prog_tests/snprintf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/snprintf.c
> @@ -116,6 +116,8 @@ 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_ERR(load_single_snprintf("%p%"), "invalid specifier 8");
> +	ASSERT_ERR(load_single_snprintf("%s%"), "invalid specifier 9");

The above "%s%" test already succeeded without Patch 1. It would be
good to mention this in the commit message to avoid confusion.

>   }
>   
>   void test_snprintf(void)