* [PATCH bpf-next v2 1/4] bpf: Limit the number of uprobes when attaching program to multiple uprobes
2023-12-13 11:25 [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node() Hou Tao
@ 2023-12-13 11:25 ` Hou Tao
2023-12-13 11:25 ` [PATCH bpf-next v2 2/4] bpf: Limit the number of kprobes when attaching program to multiple kprobes Hou Tao
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Hou Tao @ 2023-12-13 11:25 UTC (permalink / raw)
To: bpf
Cc: Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko, Song Liu,
Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
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().
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, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 774cf476a892..2d1201f7b554 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;
@@ -3342,7 +3344,7 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
uoffsets = u64_to_user_ptr(attr->link_create.uprobe_multi.offsets);
cnt = attr->link_create.uprobe_multi.cnt;
- if (!upath || !uoffsets || !cnt)
+ if (!upath || !uoffsets || !cnt || cnt > MAX_UPROBE_MULTI_CNT)
return -EINVAL;
uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH bpf-next v2 2/4] bpf: Limit the number of kprobes when attaching program to multiple kprobes
2023-12-13 11:25 [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node() Hou Tao
2023-12-13 11:25 ` [PATCH bpf-next v2 1/4] bpf: Limit the number of uprobes when attaching program to multiple uprobes Hou Tao
@ 2023-12-13 11:25 ` Hou Tao
2023-12-13 23:32 ` Andrii Nakryiko
2023-12-13 11:25 ` [PATCH bpf-next v2 3/4] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment Hou Tao
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Hou Tao @ 2023-12-13 11:25 UTC (permalink / raw)
To: bpf
Cc: Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko, Song Liu,
Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
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().
Fixes: 0dcac2725406 ("bpf: Add multi kprobe link")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
kernel/trace/bpf_trace.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 2d1201f7b554..944678529f5c 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 {
@@ -2970,7 +2971,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
return -EINVAL;
cnt = attr->link_create.kprobe_multi.cnt;
- if (!cnt)
+ if (!cnt || cnt > MAX_KPROBE_MULTI_CNT)
return -EINVAL;
size = cnt * sizeof(*addrs);
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v2 2/4] bpf: Limit the number of kprobes when attaching program to multiple kprobes
2023-12-13 11:25 ` [PATCH bpf-next v2 2/4] bpf: Limit the number of kprobes when attaching program to multiple kprobes Hou Tao
@ 2023-12-13 23:32 ` Andrii Nakryiko
2023-12-14 1:00 ` Hou Tao
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2023-12-13 23:32 UTC (permalink / raw)
To: Hou Tao
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
On Wed, Dec 13, 2023 at 3:24 AM Hou Tao <houtao@huaweicloud.com> wrote:
>
> 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().
>
> Fixes: 0dcac2725406 ("bpf: Add multi kprobe link")
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> kernel/trace/bpf_trace.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 2d1201f7b554..944678529f5c 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 {
> @@ -2970,7 +2971,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> return -EINVAL;
>
> cnt = attr->link_create.kprobe_multi.cnt;
> - if (!cnt)
> + if (!cnt || cnt > MAX_KPROBE_MULTI_CNT)
> return -EINVAL;
let's return -E2BIG for `cnt > MAX` cases? Same in another patch
>
> size = cnt * sizeof(*addrs);
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v2 2/4] bpf: Limit the number of kprobes when attaching program to multiple kprobes
2023-12-13 23:32 ` Andrii Nakryiko
@ 2023-12-14 1:00 ` Hou Tao
0 siblings, 0 replies; 13+ messages in thread
From: Hou Tao @ 2023-12-14 1:00 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
Hi,
On 12/14/2023 7:32 AM, Andrii Nakryiko wrote:
> On Wed, Dec 13, 2023 at 3:24 AM Hou Tao <houtao@huaweicloud.com> wrote:
>> 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().
>>
>> Fixes: 0dcac2725406 ("bpf: Add multi kprobe link")
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> kernel/trace/bpf_trace.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
>> index 2d1201f7b554..944678529f5c 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 {
>> @@ -2970,7 +2971,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>> return -EINVAL;
>>
>> cnt = attr->link_create.kprobe_multi.cnt;
>> - if (!cnt)
>> + if (!cnt || cnt > MAX_KPROBE_MULTI_CNT)
>> return -EINVAL;
> let's return -E2BIG for `cnt > MAX` cases? Same in another patch
Good point. Will do in v3.
>> size = cnt * sizeof(*addrs);
>> --
>> 2.29.2
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH bpf-next v2 3/4] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment
2023-12-13 11:25 [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node() Hou Tao
2023-12-13 11:25 ` [PATCH bpf-next v2 1/4] bpf: Limit the number of uprobes when attaching program to multiple uprobes Hou Tao
2023-12-13 11:25 ` [PATCH bpf-next v2 2/4] bpf: Limit the number of kprobes when attaching program to multiple kprobes Hou Tao
@ 2023-12-13 11:25 ` Hou Tao
2023-12-13 14:43 ` Jiri Olsa
2023-12-13 11:25 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment Hou Tao
2023-12-13 14:42 ` [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node() Jiri Olsa
4 siblings, 1 reply; 13+ messages in thread
From: Hou Tao @ 2023-12-13 11:25 UTC (permalink / raw)
To: bpf
Cc: Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko, Song Liu,
Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
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.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
.../bpf/prog_tests/uprobe_multi_test.c | 33 ++++++++++++++++++-
1 file changed, 32 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..0d2a4510e6cf 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,35 @@ static void test_attach_api_syms(void)
test_attach_api("/proc/self/exe", NULL, &opts);
}
+static void test_failed_link_api(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;
+ opts.kprobe_multi.flags = 0;
+ 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, -EINVAL, "invalid 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 +340,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;
@@ -408,6 +437,8 @@ void test_uprobe_multi_test(void)
test_attach_api_syms();
if (test__start_subtest("link_api"))
test_link_api();
+ if (test__start_subtest("failed_link_api"))
+ test_failed_link_api();
if (test__start_subtest("bench_uprobe"))
test_bench_attach_uprobe();
if (test__start_subtest("bench_usdt"))
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v2 3/4] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment
2023-12-13 11:25 ` [PATCH bpf-next v2 3/4] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment Hou Tao
@ 2023-12-13 14:43 ` Jiri Olsa
2023-12-14 1:02 ` Hou Tao
0 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2023-12-13 14:43 UTC (permalink / raw)
To: Hou Tao
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, John Fastabend, xingwei lee, houtao1
On Wed, Dec 13, 2023 at 07:25:30PM +0800, Hou Tao wrote:
> 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.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> .../bpf/prog_tests/uprobe_multi_test.c | 33 ++++++++++++++++++-
> 1 file changed, 32 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..0d2a4510e6cf 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,35 @@ static void test_attach_api_syms(void)
> test_attach_api("/proc/self/exe", NULL, &opts);
> }
>
> +static void test_failed_link_api(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;
> + opts.kprobe_multi.flags = 0;
s/k/u/ ^^^ .. or best just remove the line
jirka
> + 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, -EINVAL, "invalid 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 +340,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;
>
> @@ -408,6 +437,8 @@ void test_uprobe_multi_test(void)
> test_attach_api_syms();
> if (test__start_subtest("link_api"))
> test_link_api();
> + if (test__start_subtest("failed_link_api"))
> + test_failed_link_api();
> if (test__start_subtest("bench_uprobe"))
> test_bench_attach_uprobe();
> if (test__start_subtest("bench_usdt"))
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v2 3/4] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment
2023-12-13 14:43 ` Jiri Olsa
@ 2023-12-14 1:02 ` Hou Tao
0 siblings, 0 replies; 13+ messages in thread
From: Hou Tao @ 2023-12-14 1:02 UTC (permalink / raw)
To: Jiri Olsa
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, John Fastabend, xingwei lee, houtao1
On 12/13/2023 10:43 PM, Jiri Olsa wrote:
> On Wed, Dec 13, 2023 at 07:25:30PM +0800, Hou Tao wrote:
>> 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.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> .../bpf/prog_tests/uprobe_multi_test.c | 33 ++++++++++++++++++-
>> 1 file changed, 32 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..0d2a4510e6cf 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,35 @@ static void test_attach_api_syms(void)
>> test_attach_api("/proc/self/exe", NULL, &opts);
>> }
>>
>> +static void test_failed_link_api(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;
>> + opts.kprobe_multi.flags = 0;
> s/k/u/ ^^^ .. or best just remove the line
My bad. Will remove it. Thanks for the ack.
>
> jirka
>
>> + 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, -EINVAL, "invalid 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 +340,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;
>>
>> @@ -408,6 +437,8 @@ void test_uprobe_multi_test(void)
>> test_attach_api_syms();
>> if (test__start_subtest("link_api"))
>> test_link_api();
>> + if (test__start_subtest("failed_link_api"))
>> + test_failed_link_api();
>> if (test__start_subtest("bench_uprobe"))
>> test_bench_attach_uprobe();
>> if (test__start_subtest("bench_usdt"))
>> --
>> 2.29.2
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH bpf-next v2 4/4] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment
2023-12-13 11:25 [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node() Hou Tao
` (2 preceding siblings ...)
2023-12-13 11:25 ` [PATCH bpf-next v2 3/4] selftests/bpf: Add test for abnormal cnt during multi-uprobe attachment Hou Tao
@ 2023-12-13 11:25 ` Hou Tao
2023-12-13 23:33 ` Andrii Nakryiko
2023-12-13 14:42 ` [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node() Jiri Olsa
4 siblings, 1 reply; 13+ messages in thread
From: Hou Tao @ 2023-12-13 11:25 UTC (permalink / raw)
To: bpf
Cc: Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko, Song Liu,
Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
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.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
.../selftests/bpf/prog_tests/kprobe_multi_test.c | 14 ++++++++++++++
1 file changed, 14 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 4041cfa670eb..802554d4ee24 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -300,6 +300,20 @@ static void test_attach_api_fails(void)
if (!ASSERT_EQ(libbpf_get_error(link), -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);
+ if (!ASSERT_ERR_PTR(link, "fail_6"))
+ goto cleanup;
+
+ if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_6_error"))
+ goto cleanup;
+
cleanup:
bpf_link__destroy(link);
kprobe_multi__destroy(skel);
--
2.29.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment
2023-12-13 11:25 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment Hou Tao
@ 2023-12-13 23:33 ` Andrii Nakryiko
2023-12-14 1:44 ` Hou Tao
0 siblings, 1 reply; 13+ messages in thread
From: Andrii Nakryiko @ 2023-12-13 23:33 UTC (permalink / raw)
To: Hou Tao
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
On Wed, Dec 13, 2023 at 3:24 AM Hou Tao <houtao@huaweicloud.com> wrote:
>
> 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.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> .../selftests/bpf/prog_tests/kprobe_multi_test.c | 14 ++++++++++++++
> 1 file changed, 14 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 4041cfa670eb..802554d4ee24 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> @@ -300,6 +300,20 @@ static void test_attach_api_fails(void)
> if (!ASSERT_EQ(libbpf_get_error(link), -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);
> + if (!ASSERT_ERR_PTR(link, "fail_6"))
> + goto cleanup;
> +
> + if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_6_error"))
this is unreliable, store errno right after the operation before
ASSERT_xxx() macros
> + goto cleanup;
> +
> cleanup:
> bpf_link__destroy(link);
> kprobe_multi__destroy(skel);
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment
2023-12-13 23:33 ` Andrii Nakryiko
@ 2023-12-14 1:44 ` Hou Tao
2023-12-14 4:26 ` Andrii Nakryiko
0 siblings, 1 reply; 13+ messages in thread
From: Hou Tao @ 2023-12-14 1:44 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
Hi,
On 12/14/2023 7:33 AM, Andrii Nakryiko wrote:
> On Wed, Dec 13, 2023 at 3:24 AM Hou Tao <houtao@huaweicloud.com> wrote:
>> 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.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> .../selftests/bpf/prog_tests/kprobe_multi_test.c | 14 ++++++++++++++
>> 1 file changed, 14 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 4041cfa670eb..802554d4ee24 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>> @@ -300,6 +300,20 @@ static void test_attach_api_fails(void)
>> if (!ASSERT_EQ(libbpf_get_error(link), -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);
>> + if (!ASSERT_ERR_PTR(link, "fail_6"))
>> + goto cleanup;
>> +
>> + if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_6_error"))
> this is unreliable, store errno right after the operation before
> ASSERT_xxx() macros
I didn't fully follow the reason why it is unreliable. Do you mean
ASSERT_ERR_PTR() macro may overwrite errno, right ? But _CHECK() already
saves and restores errno before invoking fprintf(), so I think it is OK
to use libbpf_get_error() to get the errno here ?
>
>> + goto cleanup;
>> +
>> cleanup:
>> bpf_link__destroy(link);
>> kprobe_multi__destroy(skel);
>> --
>> 2.29.2
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment
2023-12-14 1:44 ` Hou Tao
@ 2023-12-14 4:26 ` Andrii Nakryiko
0 siblings, 0 replies; 13+ messages in thread
From: Andrii Nakryiko @ 2023-12-14 4:26 UTC (permalink / raw)
To: Hou Tao
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, Jiri Olsa, John Fastabend, xingwei lee,
houtao1
On Wed, Dec 13, 2023 at 5:44 PM Hou Tao <houtao@huaweicloud.com> wrote:
>
> Hi,
>
> On 12/14/2023 7:33 AM, Andrii Nakryiko wrote:
> > On Wed, Dec 13, 2023 at 3:24 AM Hou Tao <houtao@huaweicloud.com> wrote:
> >> 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.
> >>
> >> Signed-off-by: Hou Tao <houtao1@huawei.com>
> >> ---
> >> .../selftests/bpf/prog_tests/kprobe_multi_test.c | 14 ++++++++++++++
> >> 1 file changed, 14 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 4041cfa670eb..802554d4ee24 100644
> >> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> >> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> >> @@ -300,6 +300,20 @@ static void test_attach_api_fails(void)
> >> if (!ASSERT_EQ(libbpf_get_error(link), -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);
> >> + if (!ASSERT_ERR_PTR(link, "fail_6"))
> >> + goto cleanup;
> >> +
> >> + if (!ASSERT_EQ(libbpf_get_error(link), -EINVAL, "fail_6_error"))
> > this is unreliable, store errno right after the operation before
> > ASSERT_xxx() macros
>
> I didn't fully follow the reason why it is unreliable. Do you mean
> ASSERT_ERR_PTR() macro may overwrite errno, right ? But _CHECK() already
> saves and restores errno before invoking fprintf(), so I think it is OK
> to use libbpf_get_error() to get the errno here ?
We shouldn't be using libbpf_get_error() in new code, it's legacy and
not advised to be used. If ASSERT_xxx() macro guarantee to
save/restore errno, then it might be actually fine, but it still feels
better to save errno explicitly right after the operation, just like
you'd do in normal code.
> >
> >> + goto cleanup;
> >> +
> >> cleanup:
> >> bpf_link__destroy(link);
> >> kprobe_multi__destroy(skel);
> >> --
> >> 2.29.2
> >>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node()
2023-12-13 11:25 [PATCH bpf-next v2 0/4] bpf: Fix warnings in kvmalloc_node() Hou Tao
` (3 preceding siblings ...)
2023-12-13 11:25 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add test for abnormal cnt during multi-kprobe attachment Hou Tao
@ 2023-12-13 14:42 ` Jiri Olsa
4 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2023-12-13 14:42 UTC (permalink / raw)
To: Hou Tao
Cc: bpf, Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Song Liu, Hao Luo, Yonghong Song, Daniel Borkmann, KP Singh,
Stanislav Fomichev, John Fastabend, xingwei lee, houtao1
On Wed, Dec 13, 2023 at 07:25:27PM +0800, Hou Tao 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 and #4 add tests to ensure these warnings are
> fixed.
>
> Please see individual patches for more details. Comments are always
> welcome.
>
> Change Log:
> v2:
> * 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 (4):
> 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: Add test for abnormal cnt during multi-kprobe
> attachment
>
> kernel/trace/bpf_trace.c | 7 ++--
> .../bpf/prog_tests/kprobe_multi_test.c | 14 ++++++++
> .../bpf/prog_tests/uprobe_multi_test.c | 33 ++++++++++++++++++-
> 3 files changed, 51 insertions(+), 3 deletions(-)
with one minor comment, for the patchset
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
>
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread