public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6
@ 2026-03-18 22:20 Ibrahim Zein
  2026-03-18 22:26 ` Alexei Starovoitov
  0 siblings, 1 reply; 6+ messages in thread
From: Ibrahim Zein @ 2026-03-18 22:20 UTC (permalink / raw)
  To: ast; +Cc: daniel, martin.lau, andrii, bpf, Ibrahim Zein

From: Ibrahim Zein <zeroxjacks@gmail.com>

In bpf_bprintf_prepare(), the bounds check for %pI4 and %pI6 format
specifiers uses sizeof_cur_ip (4 for IPv4, 16 for IPv6), which is the
raw byte count of the IP address. However, snprintf() returns the
length of the formatted string, not the raw bytes. For IPv4 this can
be up to 15 characters ("255.255.255.255") and for IPv6 up to 39.

tmp_buf is then advanced by (err + 1) using the full string length,
which can push tmp_buf past tmp_buf_end. The next iteration's bounds
check underflows due to unsigned arithmetic and passes, allowing a
write past the end of the per-CPU bin_args buffer.

Fix this by checking against the maximum formatted string size instead
of the raw byte count: 16 bytes for IPv4 and 40 bytes for IPv6.

Signed-off-by: Ibrahim Zein <zeroxjacks@gmail.com>
---
--- a/kernel/bpf/helpers.c	2026-03-18 18:04:49.000000000 -0400
+++ b/kernel/bpf/helpers.c	2026-03-18 18:09:39.126681954 -0400
@@ -930,7 +930,7 @@
 				goto nocopy_fmt;
 
 			sizeof_cur_ip = (fmt[i] == '4') ? 4 : 16;
-			if (tmp_buf_end - tmp_buf < sizeof_cur_ip) {
+			if (tmp_buf_end - tmp_buf < (size_t)((fmt[i] == '4') ? 16 : 40)) {
 				err = -ENOSPC;
 				goto out;
 			}
--- a/kernel/bpf/helpers.c	2026-03-18 18:04:49.000000000 -0400
+++ b/kernel/bpf/helpers.c	2026-03-18 18:09:39.126681954 -0400
@@ -930,7 +930,7 @@
 				goto nocopy_fmt;
 
 			sizeof_cur_ip = (fmt[i] == '4') ? 4 : 16;
-			if (tmp_buf_end - tmp_buf < sizeof_cur_ip) {
+			if (tmp_buf_end - tmp_buf < (size_t)((fmt[i] == '4') ? 16 : 40)) {
 				err = -ENOSPC;
 				goto out;
 			}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6
  2026-03-18 22:20 [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6 Ibrahim Zein
@ 2026-03-18 22:26 ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2026-03-18 22:26 UTC (permalink / raw)
  To: Ibrahim Zein
  Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
	Andrii Nakryiko, bpf

On Wed, Mar 18, 2026 at 3:20 PM Ibrahim Zein <zeroxjacks@gmail.com> wrote:
>
> From: Ibrahim Zein <zeroxjacks@gmail.com>
>
> In bpf_bprintf_prepare(), the bounds check for %pI4 and %pI6 format
> specifiers uses sizeof_cur_ip (4 for IPv4, 16 for IPv6), which is the
> raw byte count of the IP address. However, snprintf() returns the
> length of the formatted string, not the raw bytes. For IPv4 this can
> be up to 15 characters ("255.255.255.255") and for IPv6 up to 39.
>
> tmp_buf is then advanced by (err + 1) using the full string length,
> which can push tmp_buf past tmp_buf_end. The next iteration's bounds
> check underflows due to unsigned arithmetic and passes, allowing a
> write past the end of the per-CPU bin_args buffer.
>
> Fix this by checking against the maximum formatted string size instead
> of the raw byte count: 16 bytes for IPv4 and 40 bytes for IPv6.
>
> Signed-off-by: Ibrahim Zein <zeroxjacks@gmail.com>
> ---
> --- a/kernel/bpf/helpers.c      2026-03-18 18:04:49.000000000 -0400
> +++ b/kernel/bpf/helpers.c      2026-03-18 18:09:39.126681954 -0400
> @@ -930,7 +930,7 @@
>                                 goto nocopy_fmt;
>
>                         sizeof_cur_ip = (fmt[i] == '4') ? 4 : 16;
> -                       if (tmp_buf_end - tmp_buf < sizeof_cur_ip) {
> +                       if (tmp_buf_end - tmp_buf < (size_t)((fmt[i] == '4') ? 16 : 40)) {

Ibrahim,

Please stop this spam.
So far you haven't sent a single valid patch and ignored
requests to provide selftests first.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6
@ 2026-03-19  3:47 Ibrahim Zein
  2026-03-19  4:29 ` bot+bpf-ci
  0 siblings, 1 reply; 6+ messages in thread
From: Ibrahim Zein @ 2026-03-19  3:47 UTC (permalink / raw)
  To: ast; +Cc: daniel, martin.lau, andrii, bpf, Ibrahim Zein

In bpf_bprintf_prepare(), the bounds check for %pI4 and %pI6 format
specifiers uses sizeof_cur_ip (4 for IPv4, 16 for IPv6), which is the
raw byte count of the IP address. However, snprintf() returns the
length of the formatted string, not the raw bytes. For IPv4 this can
be up to 15 characters (255.255.255.255) and for IPv6 up to 39.

tmp_buf is then advanced by (err + 1) using the full string length,
which can push tmp_buf past tmp_buf_end. The next iteration's bounds
check underflows due to unsigned arithmetic and passes, allowing a
write past the end of the per-CPU bin_args buffer.

Fix this by checking against the maximum formatted string size:
16 bytes for IPv4 and 40 bytes for IPv6.

Signed-off-by: Ibrahim Zein <zeroxjacks@gmail.com>
---
 kernel/bpf/helpers.c                          |  2 +-
 .../bpf/prog_tests/test_snprintf_ip.c         | 54 +++++++++++++
 .../selftests/bpf/progs/test_snprintf_ip.c    | 81 +++++++++++++++++++
 3 files changed, 136 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf_ip.c

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index cb6d242bd..dcaa822ba 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -930,7 +930,7 @@ int bpf_bprintf_prepare(const char *fmt, u32 fmt_size, const u64 *raw_args,
 				goto nocopy_fmt;
 
 			sizeof_cur_ip = (fmt[i] == '4') ? 4 : 16;
-			if (tmp_buf_end - tmp_buf < sizeof_cur_ip) {
+			if (tmp_buf_end - tmp_buf < (size_t)((fmt[i] == '4') ? 16 : 40)) {
 				err = -ENOSPC;
 				goto out;
 			}
diff --git a/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c b/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
new file mode 100644
index 000000000..5b000d6d1
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Ibrahim Zein <zeroxjacks@gmail.com> */
+#include <test_progs.h>
+#include "test_snprintf_ip.skel.h"
+
+#define EXP_IP4_OUT  "192.168.1.1"
+#define EXP_IP4_RET  sizeof(EXP_IP4_OUT)
+
+#define EXP_WORST_IP4_OUT  "255.255.255.255"
+#define EXP_WORST_IP4_RET  sizeof(EXP_WORST_IP4_OUT)
+
+#define EXP_IP6_OUT  "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"
+#define EXP_IP6_RET  sizeof(EXP_IP6_OUT)
+
+void test_snprintf_ip(void)
+{
+	struct test_snprintf_ip *skel;
+
+	skel = test_snprintf_ip__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	skel->bss->pid = getpid();
+
+	if (!ASSERT_OK(test_snprintf_ip__attach(skel), "skel_attach"))
+		goto cleanup;
+
+	/* trigger tracepoint */
+	usleep(1);
+
+	/* Test 1: normal IPv4 */
+	ASSERT_STREQ(skel->bss->ip4_out, EXP_IP4_OUT, "ip4_out");
+	ASSERT_EQ(skel->bss->ip4_ret, EXP_IP4_RET, "ip4_ret");
+
+	/* Test 2: normal IPv6 */
+	ASSERT_STREQ(skel->bss->ip6_out, EXP_IP6_OUT, "ip6_out");
+	ASSERT_EQ(skel->bss->ip6_ret, EXP_IP6_RET, "ip6_ret");
+
+	/* Test 3: worst-case IPv4 "255.255.255.255" */
+	ASSERT_STREQ(skel->bss->worst_ip4_out, EXP_WORST_IP4_OUT,
+		     "worst_ip4_out");
+	ASSERT_EQ(skel->bss->worst_ip4_ret, EXP_WORST_IP4_RET,
+		  "worst_ip4_ret");
+
+	/*
+	 * Test 4: near-overflow scenario.
+	 * Before fix: tmp_buf overflows, kernel may crash or corrupt memory.
+	 * After fix: returns valid length without overflow.
+	 */
+	ASSERT_GE(skel->bss->near_overflow_ret, 0, "near_overflow_ret");
+
+cleanup:
+	test_snprintf_ip__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_snprintf_ip.c b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c
new file mode 100644
index 000000000..44cc4d74c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Ibrahim Zein <zeroxjacks@gmail.com> */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+/*
+ * Test that bpf_snprintf() with %pI4/%pI6 does not overflow the
+ * internal bin_args buffer when the buffer is nearly full.
+ *
+ * The bug: sizeof_cur_ip (4 for IPv4) was used for the bounds check,
+ * but snprintf() returns the full formatted string length (up to 15
+ * for "255.255.255.255"), pushing tmp_buf past tmp_buf_end.
+ */
+
+/* Output buffers */
+char ip4_out[64] = {};
+long ip4_ret = 0;
+
+char ip6_out[128] = {};
+long ip6_ret = 0;
+
+/* Test %pI4 with worst-case IP (255.255.255.255) */
+char worst_ip4_out[64] = {};
+long worst_ip4_ret = 0;
+
+/* Return value for the near-overflow test */
+long near_overflow_ret = 0;
+
+__u32 pid = 0;
+
+SEC("raw_tp/sys_enter")
+int handler(const void *ctx)
+{
+	/* Worst-case IPv4: "255.255.255.255" = 15 chars */
+	const __u8 ip4_worst[] = {255, 255, 255, 255};
+	/* Normal IPv4 */
+	const __u8 ip4_normal[] = {192, 168, 1, 1};
+	/* IPv6 worst-case */
+	const __u8 ip6_worst[] = {0xff, 0xff, 0xff, 0xff,
+				   0xff, 0xff, 0xff, 0xff,
+				   0xff, 0xff, 0xff, 0xff,
+				   0xff, 0xff, 0xff, 0xff};
+
+	if ((int)bpf_get_current_pid_tgid() != pid)
+		return 0;
+
+	/* Test 1: normal %pI4 usage */
+	ip4_ret = BPF_SNPRINTF(ip4_out, sizeof(ip4_out),
+			       "%pI4", &ip4_normal);
+
+	/* Test 2: normal %pI6 usage */
+	ip6_ret = BPF_SNPRINTF(ip6_out, sizeof(ip6_out),
+			       "%pI6", &ip6_worst);
+
+	/* Test 3: worst-case %pI4 "255.255.255.255" */
+	worst_ip4_ret = BPF_SNPRINTF(worst_ip4_out, sizeof(worst_ip4_out),
+				     "%pI4", &ip4_worst);
+
+	/*
+	 * Test 4: near-overflow scenario.
+	 * Fill bin_args with scalar args (%d), then use %pI4.
+	 * Before the fix: tmp_buf overflows by 8 bytes.
+	 * After the fix: correctly returns -ENOSPC or fits exactly.
+	 *
+	 * We use fewer args here since BPF_SNPRINTF macro has a limit,
+	 * but this validates the format string parsing path.
+	 */
+	near_overflow_ret = BPF_SNPRINTF(NULL, 0,
+		"%d %d %d %d %d %d %d %d "
+		"%d %d %d %d %d %d %d %d "
+		"%d %d %d %d %d %d %d %d "
+		"%pI4",
+		1, 2, 3, 4, 5, 6, 7, 8,
+		9, 10, 11, 12, 13, 14, 15, 16,
+		17, 18, 19, 20, 21, 22, 23, 24,
+		&ip4_worst);
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6
  2026-03-19  3:47 Ibrahim Zein
@ 2026-03-19  4:29 ` bot+bpf-ci
  0 siblings, 0 replies; 6+ messages in thread
From: bot+bpf-ci @ 2026-03-19  4:29 UTC (permalink / raw)
  To: zeroxjacks, ast
  Cc: daniel, martin.lau, andrii, bpf, zeroxjacks, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]

> diff --git a/tools/testing/selftests/bpf/progs/test_snprintf_ip.c b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c
> new file mode 100644
> index 000000000000..44cc4d74c5e2
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c

[ ... ]

> +	/*
> +	 * Test 4: near-overflow scenario.
> +	 * Fill bin_args with scalar args (%d), then use %pI4.
> +	 * Before the fix: tmp_buf overflows by 8 bytes.
> +	 * After the fix: correctly returns -ENOSPC or fits exactly.
> +	 *
> +	 * We use fewer args here since BPF_SNPRINTF macro has a limit,
> +	 * but this validates the format string parsing path.
> +	 */
> +	near_overflow_ret = BPF_SNPRINTF(NULL, 0,
> +		"%d %d %d %d %d %d %d %d "
> +		"%d %d %d %d %d %d %d %d "
> +		"%d %d %d %d %d %d %d %d "
> +		"%pI4",
> +		1, 2, 3, 4, 5, 6, 7, 8,
> +		9, 10, 11, 12, 13, 14, 15, 16,
> +		17, 18, 19, 20, 21, 22, 23, 24,
> +		&ip4_worst);

The comment says "we use fewer args here since BPF_SNPRINTF macro has
a limit", but this call passes 25 variadic arguments (24 integers plus
one pointer).  The BPF_SNPRINTF macro only supports up to 12 arguments.

The ___bpf_narg() counting macro in tools/lib/bpf/bpf_helpers.h handles
at most 12 arguments, and ___bpf_fill is only defined for ___bpf_fill0
through ___bpf_fill12.  With 25 arguments, ___bpf_narg overflows and
the expansion references ___bpf_fill13, which does not exist.

Won't this fail to compile?  The argument count would need to be reduced
to at most 12 (11 scalars plus the %pI4 pointer, for example) for the
BPF program to build.

Additionally, this commit fixes an out-of-bounds write that has been
present since bpf_bprintf_prepare() was introduced but does not include
a Fixes: tag.  Should this have:

Fixes: 48cac3f4a96d ("bpf: Implement formatted output helpers with bstr_printf")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/23279168532

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6
@ 2026-03-19  4:30 Ibrahim Zein
  2026-03-19 15:23 ` Mykyta Yatsenko
  0 siblings, 1 reply; 6+ messages in thread
From: Ibrahim Zein @ 2026-03-19  4:30 UTC (permalink / raw)
  To: ast; +Cc: daniel, martin.lau, andrii, bpf, Ibrahim Zein

In bpf_bprintf_prepare(), the bounds check for %pI4 and %pI6 format
specifiers uses sizeof_cur_ip (4 for IPv4, 16 for IPv6), which is the
raw byte count of the IP address. However, snprintf() returns the
length of the formatted string, not the raw bytes. For IPv4 this can
be up to 15 characters (255.255.255.255) and for IPv6 up to 39.

tmp_buf is then advanced by (err + 1) using the full string length,
which can push tmp_buf past tmp_buf_end. The next iteration's bounds
check underflows due to unsigned arithmetic and passes, allowing a
write past the end of the per-CPU bin_args buffer.

Fix this by checking against the maximum formatted string size:
16 bytes for IPv4 and 40 bytes for IPv6.

Signed-off-by: Ibrahim Zein <zeroxjacks@gmail.com>
---
 kernel/bpf/helpers.c                          |  2 +-
 .../bpf/prog_tests/test_snprintf_ip.c         | 54 +++++++++++++
 .../selftests/bpf/progs/test_snprintf_ip.c    | 77 +++++++++++++++++++
 3 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf_ip.c

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index cb6d242bd..dcaa822ba 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -930,7 +930,7 @@ int bpf_bprintf_prepare(const char *fmt, u32 fmt_size, const u64 *raw_args,
 				goto nocopy_fmt;
 
 			sizeof_cur_ip = (fmt[i] == '4') ? 4 : 16;
-			if (tmp_buf_end - tmp_buf < sizeof_cur_ip) {
+			if (tmp_buf_end - tmp_buf < (size_t)((fmt[i] == '4') ? 16 : 40)) {
 				err = -ENOSPC;
 				goto out;
 			}
diff --git a/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c b/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
new file mode 100644
index 000000000..5b000d6d1
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Ibrahim Zein <zeroxjacks@gmail.com> */
+#include <test_progs.h>
+#include "test_snprintf_ip.skel.h"
+
+#define EXP_IP4_OUT  "192.168.1.1"
+#define EXP_IP4_RET  sizeof(EXP_IP4_OUT)
+
+#define EXP_WORST_IP4_OUT  "255.255.255.255"
+#define EXP_WORST_IP4_RET  sizeof(EXP_WORST_IP4_OUT)
+
+#define EXP_IP6_OUT  "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"
+#define EXP_IP6_RET  sizeof(EXP_IP6_OUT)
+
+void test_snprintf_ip(void)
+{
+	struct test_snprintf_ip *skel;
+
+	skel = test_snprintf_ip__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	skel->bss->pid = getpid();
+
+	if (!ASSERT_OK(test_snprintf_ip__attach(skel), "skel_attach"))
+		goto cleanup;
+
+	/* trigger tracepoint */
+	usleep(1);
+
+	/* Test 1: normal IPv4 */
+	ASSERT_STREQ(skel->bss->ip4_out, EXP_IP4_OUT, "ip4_out");
+	ASSERT_EQ(skel->bss->ip4_ret, EXP_IP4_RET, "ip4_ret");
+
+	/* Test 2: normal IPv6 */
+	ASSERT_STREQ(skel->bss->ip6_out, EXP_IP6_OUT, "ip6_out");
+	ASSERT_EQ(skel->bss->ip6_ret, EXP_IP6_RET, "ip6_ret");
+
+	/* Test 3: worst-case IPv4 "255.255.255.255" */
+	ASSERT_STREQ(skel->bss->worst_ip4_out, EXP_WORST_IP4_OUT,
+		     "worst_ip4_out");
+	ASSERT_EQ(skel->bss->worst_ip4_ret, EXP_WORST_IP4_RET,
+		  "worst_ip4_ret");
+
+	/*
+	 * Test 4: near-overflow scenario.
+	 * Before fix: tmp_buf overflows, kernel may crash or corrupt memory.
+	 * After fix: returns valid length without overflow.
+	 */
+	ASSERT_GE(skel->bss->near_overflow_ret, 0, "near_overflow_ret");
+
+cleanup:
+	test_snprintf_ip__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_snprintf_ip.c b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c
new file mode 100644
index 000000000..5471a2b8b
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Ibrahim Zein <zeroxjacks@gmail.com> */
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+/*
+ * Test that bpf_snprintf() with %pI4/%pI6 does not overflow the
+ * internal bin_args buffer when the buffer is nearly full.
+ *
+ * The bug: sizeof_cur_ip (4 for IPv4) was used for the bounds check,
+ * but snprintf() returns the full formatted string length (up to 15
+ * for "255.255.255.255"), pushing tmp_buf past tmp_buf_end.
+ */
+
+/* Output buffers */
+char ip4_out[64] = {};
+long ip4_ret = 0;
+
+char ip6_out[128] = {};
+long ip6_ret = 0;
+
+/* Test %pI4 with worst-case IP (255.255.255.255) */
+char worst_ip4_out[64] = {};
+long worst_ip4_ret = 0;
+
+/* Return value for the near-overflow test */
+long near_overflow_ret = 0;
+
+__u32 pid = 0;
+
+SEC("raw_tp/sys_enter")
+int handler(const void *ctx)
+{
+	/* Worst-case IPv4: "255.255.255.255" = 15 chars */
+	const __u8 ip4_worst[] = {255, 255, 255, 255};
+	/* Normal IPv4 */
+	const __u8 ip4_normal[] = {192, 168, 1, 1};
+	/* IPv6 worst-case */
+	const __u8 ip6_worst[] = {0xff, 0xff, 0xff, 0xff,
+				   0xff, 0xff, 0xff, 0xff,
+				   0xff, 0xff, 0xff, 0xff,
+				   0xff, 0xff, 0xff, 0xff};
+
+	if ((int)bpf_get_current_pid_tgid() != pid)
+		return 0;
+
+	/* Test 1: normal %pI4 usage */
+	ip4_ret = BPF_SNPRINTF(ip4_out, sizeof(ip4_out),
+			       "%pI4", &ip4_normal);
+
+	/* Test 2: normal %pI6 usage */
+	ip6_ret = BPF_SNPRINTF(ip6_out, sizeof(ip6_out),
+			       "%pI6", &ip6_worst);
+
+	/* Test 3: worst-case %pI4 "255.255.255.255" */
+	worst_ip4_ret = BPF_SNPRINTF(worst_ip4_out, sizeof(worst_ip4_out),
+				     "%pI4", &ip4_worst);
+
+	/*
+	 * Test 4: near-overflow scenario.
+	 * Fill bin_args with scalar args (%d), then use %pI4.
+	 * Before the fix: tmp_buf overflows by 8 bytes.
+	 * After the fix: correctly returns -ENOSPC or fits exactly.
+	 *
+	 * We use fewer args here since BPF_SNPRINTF macro has a limit,
+	 * but this validates the format string parsing path.
+	 */
+	/* 11 x %d + 1 x %pI4 = 12 args (BPF_SNPRINTF max) */
+	near_overflow_ret = BPF_SNPRINTF(NULL, 0,
+		"%d %d %d %d %d %d %d %d %d %d %d %pI4",
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,
+		&ip4_worst);
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6
  2026-03-19  4:30 Ibrahim Zein
@ 2026-03-19 15:23 ` Mykyta Yatsenko
  0 siblings, 0 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2026-03-19 15:23 UTC (permalink / raw)
  To: Ibrahim Zein, ast; +Cc: daniel, martin.lau, andrii, bpf, Ibrahim Zein

Ibrahim Zein <zeroxjacks@gmail.com> writes:

> In bpf_bprintf_prepare(), the bounds check for %pI4 and %pI6 format
> specifiers uses sizeof_cur_ip (4 for IPv4, 16 for IPv6), which is the
> raw byte count of the IP address. However, snprintf() returns the
> length of the formatted string, not the raw bytes. For IPv4 this can
> be up to 15 characters (255.255.255.255) and for IPv6 up to 39.
>
> tmp_buf is then advanced by (err + 1) using the full string length,
> which can push tmp_buf past tmp_buf_end. The next iteration's bounds
> check underflows due to unsigned arithmetic and passes, allowing a
> write past the end of the per-CPU bin_args buffer.
>
> Fix this by checking against the maximum formatted string size:
> 16 bytes for IPv4 and 40 bytes for IPv6.
>
> Signed-off-by: Ibrahim Zein <zeroxjacks@gmail.com>
> ---
>  kernel/bpf/helpers.c                          |  2 +-
>  .../bpf/prog_tests/test_snprintf_ip.c         | 54 +++++++++++++
>  .../selftests/bpf/progs/test_snprintf_ip.c    | 77 +++++++++++++++++++
>  3 files changed, 132 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf_ip.c
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index cb6d242bd..dcaa822ba 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -930,7 +930,7 @@ int bpf_bprintf_prepare(const char *fmt, u32 fmt_size, const u64 *raw_args,
>  				goto nocopy_fmt;
>  
>  			sizeof_cur_ip = (fmt[i] == '4') ? 4 : 16;
> -			if (tmp_buf_end - tmp_buf < sizeof_cur_ip) {
> +			if (tmp_buf_end - tmp_buf < (size_t)((fmt[i] == '4') ? 16 : 40)) {
not sure if casting to size_it is needed here.
>  				err = -ENOSPC;
>  				goto out;
>  			}
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c b/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
> new file mode 100644
> index 000000000..5b000d6d1
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/test_snprintf_ip.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Ibrahim Zein <zeroxjacks@gmail.com> */
> +#include <test_progs.h>
> +#include "test_snprintf_ip.skel.h"
> +
> +#define EXP_IP4_OUT  "192.168.1.1"
> +#define EXP_IP4_RET  sizeof(EXP_IP4_OUT)
> +
> +#define EXP_WORST_IP4_OUT  "255.255.255.255"
> +#define EXP_WORST_IP4_RET  sizeof(EXP_WORST_IP4_OUT)
> +
> +#define EXP_IP6_OUT  "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"
> +#define EXP_IP6_RET  sizeof(EXP_IP6_OUT)
> +
> +void test_snprintf_ip(void)
The test passes with or without the fix, I suggest to rewrite it to fail
on the base revision, otherwise it is not very useful.
> +{
> +	struct test_snprintf_ip *skel;
> +
> +	skel = test_snprintf_ip__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "skel_open"))
> +		return;
> +
> +	skel->bss->pid = getpid();
> +
> +	if (!ASSERT_OK(test_snprintf_ip__attach(skel), "skel_attach"))
> +		goto cleanup;
> +
> +	/* trigger tracepoint */
> +	usleep(1);
> +
> +	/* Test 1: normal IPv4 */
> +	ASSERT_STREQ(skel->bss->ip4_out, EXP_IP4_OUT, "ip4_out");
> +	ASSERT_EQ(skel->bss->ip4_ret, EXP_IP4_RET, "ip4_ret");
> +
> +	/* Test 2: normal IPv6 */
> +	ASSERT_STREQ(skel->bss->ip6_out, EXP_IP6_OUT, "ip6_out");
> +	ASSERT_EQ(skel->bss->ip6_ret, EXP_IP6_RET, "ip6_ret");
> +
> +	/* Test 3: worst-case IPv4 "255.255.255.255" */
> +	ASSERT_STREQ(skel->bss->worst_ip4_out, EXP_WORST_IP4_OUT,
> +		     "worst_ip4_out");
> +	ASSERT_EQ(skel->bss->worst_ip4_ret, EXP_WORST_IP4_RET,
> +		  "worst_ip4_ret");
> +
> +	/*
> +	 * Test 4: near-overflow scenario.
> +	 * Before fix: tmp_buf overflows, kernel may crash or corrupt memory.
> +	 * After fix: returns valid length without overflow.
> +	 */
> +	ASSERT_GE(skel->bss->near_overflow_ret, 0, "near_overflow_ret");
> +
> +cleanup:
> +	test_snprintf_ip__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_snprintf_ip.c b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c
> new file mode 100644
> index 000000000..5471a2b8b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_snprintf_ip.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Ibrahim Zein <zeroxjacks@gmail.com> */
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +/*
> + * Test that bpf_snprintf() with %pI4/%pI6 does not overflow the
> + * internal bin_args buffer when the buffer is nearly full.
> + *
> + * The bug: sizeof_cur_ip (4 for IPv4) was used for the bounds check,
> + * but snprintf() returns the full formatted string length (up to 15
> + * for "255.255.255.255"), pushing tmp_buf past tmp_buf_end.
> + */
> +
> +/* Output buffers */
> +char ip4_out[64] = {};
> +long ip4_ret = 0;
> +
> +char ip6_out[128] = {};
> +long ip6_ret = 0;
> +
> +/* Test %pI4 with worst-case IP (255.255.255.255) */
> +char worst_ip4_out[64] = {};
> +long worst_ip4_ret = 0;
> +
> +/* Return value for the near-overflow test */
> +long near_overflow_ret = 0;
> +
> +__u32 pid = 0;
> +
> +SEC("raw_tp/sys_enter")
> +int handler(const void *ctx)
> +{
> +	/* Worst-case IPv4: "255.255.255.255" = 15 chars */
> +	const __u8 ip4_worst[] = {255, 255, 255, 255};
> +	/* Normal IPv4 */
> +	const __u8 ip4_normal[] = {192, 168, 1, 1};
> +	/* IPv6 worst-case */
> +	const __u8 ip6_worst[] = {0xff, 0xff, 0xff, 0xff,
> +				   0xff, 0xff, 0xff, 0xff,
> +				   0xff, 0xff, 0xff, 0xff,
> +				   0xff, 0xff, 0xff, 0xff};
> +
> +	if ((int)bpf_get_current_pid_tgid() != pid)
> +		return 0;
> +
> +	/* Test 1: normal %pI4 usage */
> +	ip4_ret = BPF_SNPRINTF(ip4_out, sizeof(ip4_out),
> +			       "%pI4", &ip4_normal);
> +
> +	/* Test 2: normal %pI6 usage */
> +	ip6_ret = BPF_SNPRINTF(ip6_out, sizeof(ip6_out),
> +			       "%pI6", &ip6_worst);
> +
> +	/* Test 3: worst-case %pI4 "255.255.255.255" */
> +	worst_ip4_ret = BPF_SNPRINTF(worst_ip4_out, sizeof(worst_ip4_out),
> +				     "%pI4", &ip4_worst);
> +
> +	/*
> +	 * Test 4: near-overflow scenario.
> +	 * Fill bin_args with scalar args (%d), then use %pI4.
> +	 * Before the fix: tmp_buf overflows by 8 bytes.
> +	 * After the fix: correctly returns -ENOSPC or fits exactly.
> +	 *
> +	 * We use fewer args here since BPF_SNPRINTF macro has a limit,
> +	 * but this validates the format string parsing path.
> +	 */
> +	/* 11 x %d + 1 x %pI4 = 12 args (BPF_SNPRINTF max) */
> +	near_overflow_ret = BPF_SNPRINTF(NULL, 0,
> +		"%d %d %d %d %d %d %d %d %d %d %d %pI4",
> +		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,
> +		&ip4_worst);
> +
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> -- 
> 2.52.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-19 15:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-18 22:20 [PATCH] bpf: fix out-of-bounds write in bpf_bprintf_prepare with %pI4/%pI6 Ibrahim Zein
2026-03-18 22:26 ` Alexei Starovoitov
  -- strict thread matches above, loose matches on Subject: below --
2026-03-19  3:47 Ibrahim Zein
2026-03-19  4:29 ` bot+bpf-ci
2026-03-19  4:30 Ibrahim Zein
2026-03-19 15:23 ` Mykyta Yatsenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox