[PATCH bpf-next v3 0/5] bpf: Fix warnings in kvmalloc_node()

[PATCH bpf-next v3 0/5] bpf: Fix warnings in kvmalloc_node()

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

Hi,

The patch set aims to fix the warnings in kvmalloc_node() when passing
an abnormally big cnt during multiple kprobes/uprobes attachment.

Patch #1 and #2 fix the warning by limiting the maximal number of
uprobes/kprobes. Patch #3, #4, and #5 add tests to ensure these
warnings are fixed.

Please see individual patches for more details. Comments are always
welcome.

Change Log:
v3:
  * add ack tags from Jiri
  * return -E2BIG instead of -EINVAL for too-big cnt (Andrii)
  * patch #3: rename the subtest from "failed_link_api" to
              "attach_api_fails", so it is consistent with the naming
	      convention in multi-kprobe test.
  * patch #4: newly-added patch to remove libbpf_get_error() in
              kprobe_multi_test (Andrii)

v2: https://lore.kernel.org/bpf/20231213112531.3775079-1-houtao@huaweicloud.com/
  * limit the number of uprobes/kprobes instead of suppressing the
    out-of-memory warning message (Alexei)
  * provide a faked non-zero offsets to simplify the multiple uprobe
    test (Jiri)

v1: https://lore.kernel.org/bpf/20231211112843.4147157-1-houtao@huaweicloud.com/

Hou Tao (5):
  bpf: Limit the number of uprobes when attaching program to multiple
    uprobes
  bpf: Limit the number of kprobes when attaching program to multiple
    kprobes
  selftests/bpf: Add test for abnormal cnt during multi-uprobe
    attachment
  selftests/bpf: Don't use libbpf_get_error() in kprobe_multi_test
  selftests/bpf: Add test for abnormal cnt during multi-kprobe
    attachment

 kernel/trace/bpf_trace.c                      |  7 ++++
 .../bpf/prog_tests/kprobe_multi_test.c        | 31 +++++++++++++++---
 .../bpf/prog_tests/uprobe_multi_test.c        | 32 ++++++++++++++++++-
 3 files changed, 64 insertions(+), 6 deletions(-)

-- 
2.29.2

[PATCH bpf-next v3 1/5] bpf: Limit the number of uprobes when attaching program to multiple uprobes

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

An abnormally big cnt may be passed to link_create.uprobe_multi.cnt,
and it will trigger the following warning in kvmalloc_node():

	if (unlikely(size > INT_MAX)) {
		WARN_ON_ONCE(!(flags & __GFP_NOWARN));
		return NULL;
	}

Fix the warning by limiting the maximal number of uprobes in
bpf_uprobe_multi_link_attach(). If the number of uprobes is greater than
MAX_UPROBE_MULTI_CNT, the attachment will return -E2BIG.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Fixes: 89ae89f53d20 ("bpf: Add multi uprobe link")
Reported-by: xingwei lee <xrivendell7@gmail.com>
Closes: https://lore.kernel.org/bpf/CABOYnLwwJY=yFAGie59LFsUsBAgHfroVqbzZ5edAXbFE3YiNVA@mail.gmail.com
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 kernel/trace/bpf_trace.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 774cf476a892..75c05aea9fd9 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -42,6 +42,8 @@
 #define bpf_event_rcu_dereference(p)					\
 	rcu_dereference_protected(p, lockdep_is_held(&bpf_event_mutex))
 
+#define MAX_UPROBE_MULTI_CNT (1U << 20)
+
 #ifdef CONFIG_MODULES
 struct bpf_trace_module {
 	struct module *module;
@@ -3344,6 +3346,8 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 
 	if (!upath || !uoffsets || !cnt)
 		return -EINVAL;
+	if (cnt > MAX_UPROBE_MULTI_CNT)
+		return -E2BIG;
 
 	uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
 	ucookies = u64_to_user_ptr(attr->link_create.uprobe_multi.cookies);
-- 
2.29.2

[PATCH bpf-next v3 2/5] bpf: Limit the number of kprobes when attaching program to multiple kprobes

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

An abnormally big cnt may also be assigned to kprobe_multi.cnt when
attaching multiple kprobes. It will trigger the following warning in
kvmalloc_node():

	if (unlikely(size > INT_MAX)) {
	    WARN_ON_ONCE(!(flags & __GFP_NOWARN));
	    return NULL;
	}

Fix the warning by limiting the maximal number of kprobes in
bpf_kprobe_multi_link_attach(). If the number of kprobes is greater than
MAX_KPROBE_MULTI_CNT, the attachment will fail and return -E2BIG.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Fixes: 0dcac2725406 ("bpf: Add multi kprobe link")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 kernel/trace/bpf_trace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 75c05aea9fd9..97c0c49c40a0 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -43,6 +43,7 @@
 	rcu_dereference_protected(p, lockdep_is_held(&bpf_event_mutex))
 
 #define MAX_UPROBE_MULTI_CNT (1U << 20)
+#define MAX_KPROBE_MULTI_CNT (1U << 20)
 
 #ifdef CONFIG_MODULES
 struct bpf_trace_module {
@@ -2972,6 +2973,8 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	cnt = attr->link_create.kprobe_multi.cnt;
 	if (!cnt)
 		return -EINVAL;
+	if (cnt > MAX_KPROBE_MULTI_CNT)
+		return -E2BIG;
 
 	size = cnt * sizeof(*addrs);
 	addrs = kvmalloc_array(cnt, sizeof(*addrs), GFP_KERNEL);
-- 
2.29.2

[PATCH bpf-next v3 3/5] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

If an abnormally huge cnt is used for multi-uprobes attachment, the
following warning will be reported:

  ------------[ cut here ]------------
  WARNING: CPU: 7 PID: 406 at mm/util.c:632 kvmalloc_node+0xd9/0xe0
  Modules linked in: bpf_testmod(O)
  CPU: 7 PID: 406 Comm: test_progs Tainted: G ...... 6.7.0-rc3+ #32
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) ......
  RIP: 0010:kvmalloc_node+0xd9/0xe0
  ......
  Call Trace:
   <TASK>
   ? __warn+0x89/0x150
   ? kvmalloc_node+0xd9/0xe0
   bpf_uprobe_multi_link_attach+0x14a/0x480
   __sys_bpf+0x14a9/0x2bc0
   do_syscall_64+0x36/0xb0
   entry_SYSCALL_64_after_hwframe+0x6e/0x76
   ......
   </TASK>
  ---[ end trace 0000000000000000 ]---

So add a test to ensure the warning is fixed.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 .../bpf/prog_tests/uprobe_multi_test.c        | 32 ++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index ece260cf2c0b..07a009f95e85 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -234,6 +234,34 @@ static void test_attach_api_syms(void)
 	test_attach_api("/proc/self/exe", NULL, &opts);
 }
 
+static void test_attach_api_fails(void)
+{
+	LIBBPF_OPTS(bpf_link_create_opts, opts);
+	const char *path = "/proc/self/exe";
+	struct uprobe_multi *skel = NULL;
+	unsigned long offset = 0;
+	int link_fd = -1;
+
+	skel = uprobe_multi__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "uprobe_multi__open_and_load"))
+		goto cleanup;
+
+	/* abnormal cnt */
+	opts.uprobe_multi.path = path;
+	opts.uprobe_multi.offsets = &offset;
+	opts.uprobe_multi.cnt = INT_MAX;
+	link_fd = bpf_link_create(bpf_program__fd(skel->progs.uprobe), 0,
+				  BPF_TRACE_UPROBE_MULTI, &opts);
+	if (!ASSERT_ERR(link_fd, "link_fd"))
+		goto cleanup;
+	if (!ASSERT_EQ(link_fd, -E2BIG, "big cnt"))
+		goto cleanup;
+cleanup:
+	if (link_fd >= 0)
+		close(link_fd);
+	uprobe_multi__destroy(skel);
+}
+
 static void __test_link_api(struct child *child)
 {
 	int prog_fd, link1_fd = -1, link2_fd = -1, link3_fd = -1, link4_fd = -1;
@@ -311,7 +339,7 @@ static void __test_link_api(struct child *child)
 	free(offsets);
 }
 
-void test_link_api(void)
+static void test_link_api(void)
 {
 	struct child *child;
 
@@ -412,4 +440,6 @@ void test_uprobe_multi_test(void)
 		test_bench_attach_uprobe();
 	if (test__start_subtest("bench_usdt"))
 		test_bench_attach_usdt();
+	if (test__start_subtest("attach_api_fails"))
+		test_attach_api_fails();
 }
-- 
2.29.2

[PATCH bpf-next v3 4/5] selftests/bpf: Don't use libbpf_get_error() in kprobe_multi_test

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

Since libbpf v1.0, libbpf doesn't return error code embedded into the
pointer iteself, libbpf_get_error() is deprecated and it is basically
the same as using -errno directly.

So replace the invocations of libbpf_get_error() by -errno in
kprobe_multi_test. For libbpf_get_error() in test_attach_api_fails(),
saving -errno before invoking ASSERT_xx() macros just in case that
errno is overwritten by these macros. However, the invocation of
libbpf_get_error() in get_syms() should be kept intact, because
hashmap__new() still returns a pointer with embedded error code.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 .../selftests/bpf/prog_tests/kprobe_multi_test.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 4041cfa670eb..6079611b5df4 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -222,6 +222,7 @@ static void test_attach_api_fails(void)
 		"bpf_fentry_test2",
 	};
 	__u64 cookies[2];
+	int saved_error;
 
 	addrs[0] = ksym_get_addr("bpf_fentry_test1");
 	addrs[1] = ksym_get_addr("bpf_fentry_test2");
@@ -238,10 +239,11 @@ static void test_attach_api_fails(void)
 	/* fail_1 - pattern and opts NULL */
 	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     NULL, NULL);
+	saved_error = -errno;
 	if (!ASSERT_ERR_PTR(link, "fail_1"))
 		goto cleanup;
 
-	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_1_error"))
+	if (!ASSERT_EQ(saved_error, -EINVAL, "fail_1_error"))
 		goto cleanup;
 
 	/* fail_2 - both addrs and syms set */
@@ -252,10 +254,11 @@ static void test_attach_api_fails(void)
 
 	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     NULL, &opts);
+	saved_error = -errno;
 	if (!ASSERT_ERR_PTR(link, "fail_2"))
 		goto cleanup;
 
-	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_2_error"))
+	if (!ASSERT_EQ(saved_error, -EINVAL, "fail_2_error"))
 		goto cleanup;
 
 	/* fail_3 - pattern and addrs set */
@@ -266,10 +269,11 @@ static void test_attach_api_fails(void)
 
 	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     "ksys_*", &opts);
+	saved_error = -errno;
 	if (!ASSERT_ERR_PTR(link, "fail_3"))
 		goto cleanup;
 
-	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_3_error"))
+	if (!ASSERT_EQ(saved_error, -EINVAL, "fail_3_error"))
 		goto cleanup;
 
 	/* fail_4 - pattern and cnt set */
@@ -280,10 +284,11 @@ static void test_attach_api_fails(void)
 
 	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     "ksys_*", &opts);
+	saved_error = -errno;
 	if (!ASSERT_ERR_PTR(link, "fail_4"))
 		goto cleanup;
 
-	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_4_error"))
+	if (!ASSERT_EQ(saved_error, -EINVAL, "fail_4_error"))
 		goto cleanup;
 
 	/* fail_5 - pattern and cookies */
@@ -294,10 +299,11 @@ static void test_attach_api_fails(void)
 
 	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
 						     "ksys_*", &opts);
+	saved_error = -errno;
 	if (!ASSERT_ERR_PTR(link, "fail_5"))
 		goto cleanup;
 
-	if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_5_error"))
+	if (!ASSERT_EQ(saved_error, -EINVAL, "fail_5_error"))
 		goto cleanup;
 
 cleanup:
-- 
2.29.2

[PATCH bpf-next v3 5/5] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

If an abnormally huge cnt is used for multi-kprobes attachment, the
following warning will be reported:

  ------------[ cut here ]------------
  WARNING: CPU: 1 PID: 392 at mm/util.c:632 kvmalloc_node+0xd9/0xe0
  Modules linked in: bpf_testmod(O)
  CPU: 1 PID: 392 Comm: test_progs Tainted: G ...... 6.7.0-rc3+ #32
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
  ......
  RIP: 0010:kvmalloc_node+0xd9/0xe0
   ? __warn+0x89/0x150
   ? kvmalloc_node+0xd9/0xe0
   bpf_kprobe_multi_link_attach+0x87/0x670
   __sys_bpf+0x2a28/0x2bc0
   __x64_sys_bpf+0x1a/0x30
   do_syscall_64+0x36/0xb0
   entry_SYSCALL_64_after_hwframe+0x6e/0x76
  RIP: 0033:0x7fbe067f0e0d
  ......
   </TASK>
  ---[ end trace 0000000000000000 ]---

So add a test to ensure the warning is fixed.

Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 .../selftests/bpf/prog_tests/kprobe_multi_test.c  | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 6079611b5df4..05000810e28e 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -306,6 +306,21 @@ static void test_attach_api_fails(void)
 	if (!ASSERT_EQ(saved_error, -EINVAL, "fail_5_error"))
 		goto cleanup;
 
+	/* fail_6 - abnormal cnt */
+	opts.addrs = (const unsigned long *) addrs;
+	opts.syms = NULL;
+	opts.cnt = INT_MAX;
+	opts.cookies = NULL;
+
+	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_manual,
+						     NULL, &opts);
+	saved_error = -errno;
+	if (!ASSERT_ERR_PTR(link, "fail_6"))
+		goto cleanup;
+
+	if (!ASSERT_EQ(saved_error, -E2BIG, "fail_6_error"))
+		goto cleanup;
+
 cleanup:
 	bpf_link__destroy(link);
 	kprobe_multi__destroy(skel);
-- 
2.29.2

Re: [PATCH bpf-next v3 0/5] bpf: Fix warnings in kvmalloc_node()

[Andrii Nakryiko]

On Fri, Dec 15, 2023 at 2:06 AM Hou Tao <houtao@huaweicloud.com> wrote:
>
> From: Hou Tao <houtao1@huawei.com>
>
> Hi,
>
> The patch set aims to fix the warnings in kvmalloc_node() when passing
> an abnormally big cnt during multiple kprobes/uprobes attachment.
>
> Patch #1 and #2 fix the warning by limiting the maximal number of
> uprobes/kprobes. Patch #3, #4, and #5 add tests to ensure these
> warnings are fixed.
>
> Please see individual patches for more details. Comments are always
> welcome.
>
> Change Log:
> v3:
>   * add ack tags from Jiri
>   * return -E2BIG instead of -EINVAL for too-big cnt (Andrii)
>   * patch #3: rename the subtest from "failed_link_api" to
>               "attach_api_fails", so it is consistent with the naming
>               convention in multi-kprobe test.
>   * patch #4: newly-added patch to remove libbpf_get_error() in
>               kprobe_multi_test (Andrii)
>
> v2: https://lore.kernel.org/bpf/20231213112531.3775079-1-houtao@huaweicloud.com/
>   * limit the number of uprobes/kprobes instead of suppressing the
>     out-of-memory warning message (Alexei)
>   * provide a faked non-zero offsets to simplify the multiple uprobe
>     test (Jiri)
>
> v1: https://lore.kernel.org/bpf/20231211112843.4147157-1-houtao@huaweicloud.com/
>
> Hou Tao (5):
>   bpf: Limit the number of uprobes when attaching program to multiple
>     uprobes
>   bpf: Limit the number of kprobes when attaching program to multiple
>     kprobes
>   selftests/bpf: Add test for abnormal cnt during multi-uprobe
>     attachment
>   selftests/bpf: Don't use libbpf_get_error() in kprobe_multi_test
>   selftests/bpf: Add test for abnormal cnt during multi-kprobe
>     attachment
>
>  kernel/trace/bpf_trace.c                      |  7 ++++
>  .../bpf/prog_tests/kprobe_multi_test.c        | 31 +++++++++++++++---
>  .../bpf/prog_tests/uprobe_multi_test.c        | 32 ++++++++++++++++++-
>  3 files changed, 64 insertions(+), 6 deletions(-)
>
> --
> 2.29.2
>
>

LGTM, for the series:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

Re: [PATCH bpf-next v3 0/5] bpf: Fix warnings in kvmalloc_node()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Fri, 15 Dec 2023 18:07:03 +0800 you wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> Hi,
> 
> The patch set aims to fix the warnings in kvmalloc_node() when passing
> an abnormally big cnt during multiple kprobes/uprobes attachment.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3,1/5] bpf: Limit the number of uprobes when attaching program to multiple uprobes
    https://git.kernel.org/bpf/bpf-next/c/8b2efe51ba85
  - [bpf-next,v3,2/5] bpf: Limit the number of kprobes when attaching program to multiple kprobes
    https://git.kernel.org/bpf/bpf-next/c/d6d1e6c17cab
  - [bpf-next,v3,3/5] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment
    https://git.kernel.org/bpf/bpf-next/c/0d83786f5661
  - [bpf-next,v3,4/5] selftests/bpf: Don't use libbpf_get_error() in kprobe_multi_test
    https://git.kernel.org/bpf/bpf-next/c/00cdcd2900bd
  - [bpf-next,v3,5/5] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment
    https://git.kernel.org/bpf/bpf-next/c/1467affd16b2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html