* [PATCH bpf-next v4 0/2] selftests: add test for kfunc call
@ 2022-02-06 4:31 Hou Tao
2022-02-06 4:31 ` [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test Hou Tao
2022-02-06 4:31 ` [PATCH bpf-next v4 2/2] selftests/bpf: check whether s32 is sufficient for kfunc offset Hou Tao
0 siblings, 2 replies; 7+ messages in thread
From: Hou Tao @ 2022-02-06 4:31 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Yonghong Song,
Andrii Nakryiko, David S . Miller, Jakub Kicinski,
John Fastabend, netdev, bpf, houtao1
Hi,
The patchset add a test for kfunc call to ensure s32 is sufficient for
kfunc offset. Patch #1 unexports the subtests in ksyms_module.c to fix
the confusion in test output and patch #2 adds a test in ksyms_module.c
to ensure s32 is sufficient for kfunc offset.
Change Log:
v4:
* remove merged patch "bpf, arm64: enable kfunc call"
* rebased on bpf-next
v3: https://lore.kernel.org/bpf/20220130092917.14544-1-hotforest@gmail.com
* patch #2: newly-addded to unexport unnecessary subtests
* patch #3: use kallsyms_find() instead of reimplementing it.
* patch #3: ensure kfunc call is supported before checking
whether s32 will be overflowed or not.
v2: https://lore.kernel.org/bpf/20220127071532.384888-1-houtao1@huawei.com
* add a test to check whether imm will be overflowed for kfunc call
v1: https://lore.kernel.org/bpf/20220119144942.305568-1-houtao1@huawei.com
Hou Tao (2):
selftests/bpf: do not export subtest as standalone test
selftests/bpf: check whether s32 is sufficient for kfunc offset
.../selftests/bpf/prog_tests/ksyms_module.c | 46 ++++++++++++++++++-
.../bpf/prog_tests/xdp_adjust_frags.c | 6 ---
.../bpf/prog_tests/xdp_adjust_tail.c | 4 +-
.../bpf/prog_tests/xdp_cpumap_attach.c | 4 +-
.../bpf/prog_tests/xdp_devmap_attach.c | 2 +-
5 files changed, 49 insertions(+), 13 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test
2022-02-06 4:31 [PATCH bpf-next v4 0/2] selftests: add test for kfunc call Hou Tao
@ 2022-02-06 4:31 ` Hou Tao
2022-02-07 18:17 ` Yonghong Song
2022-02-06 4:31 ` [PATCH bpf-next v4 2/2] selftests/bpf: check whether s32 is sufficient for kfunc offset Hou Tao
1 sibling, 1 reply; 7+ messages in thread
From: Hou Tao @ 2022-02-06 4:31 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Yonghong Song,
Andrii Nakryiko, David S . Miller, Jakub Kicinski,
John Fastabend, netdev, bpf, houtao1
Two subtests in ksyms_module.c are not qualified as static, so these
subtests are exported as standalone tests in tests.h and lead to
confusion for the output of "./test_progs -t ksyms_module".
By using the following command:
grep "^void \(serial_\)\?test_[a-zA-Z0-9_]\+(\(void\)\?)" \
tools/testing/selftests/bpf/prog_tests/*.c | \
awk -F : '{print $1}' | sort | uniq -c | awk '$1 != 1'
Find out that other tests also have the similar problem, so fix
these tests by marking subtests in these tests as static. For
xdp_adjust_frags.c, there is just one subtest, so just export
the subtest directly.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
tools/testing/selftests/bpf/prog_tests/ksyms_module.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c | 6 ------
tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c | 4 ++--
tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c | 2 +-
5 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
index ecc58c9e7631..a1ebac70ec29 100644
--- a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
@@ -6,7 +6,7 @@
#include "test_ksyms_module.lskel.h"
#include "test_ksyms_module.skel.h"
-void test_ksyms_module_lskel(void)
+static void test_ksyms_module_lskel(void)
{
struct test_ksyms_module_lskel *skel;
int err;
@@ -33,7 +33,7 @@ void test_ksyms_module_lskel(void)
test_ksyms_module_lskel__destroy(skel);
}
-void test_ksyms_module_libbpf(void)
+static void test_ksyms_module_libbpf(void)
{
struct test_ksyms_module *skel;
int err;
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
index 134d0ac32f59..fc2d8fa8dac5 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
@@ -102,9 +102,3 @@ void test_xdp_update_frags(void)
out:
bpf_object__close(obj);
}
-
-void test_xdp_adjust_frags(void)
-{
- if (test__start_subtest("xdp_adjust_frags"))
- test_xdp_update_frags();
-}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
index 528a8c387720..21ceac24e174 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
@@ -133,7 +133,7 @@ static void test_xdp_adjust_tail_grow2(void)
bpf_object__close(obj);
}
-void test_xdp_adjust_frags_tail_shrink(void)
+static void test_xdp_adjust_frags_tail_shrink(void)
{
const char *file = "./test_xdp_adjust_tail_shrink.o";
__u32 exp_size;
@@ -200,7 +200,7 @@ void test_xdp_adjust_frags_tail_shrink(void)
bpf_object__close(obj);
}
-void test_xdp_adjust_frags_tail_grow(void)
+static void test_xdp_adjust_frags_tail_grow(void)
{
const char *file = "./test_xdp_adjust_tail_grow.o";
__u32 exp_size;
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
index b353e1f3acb5..f775a1613833 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c
@@ -8,7 +8,7 @@
#define IFINDEX_LO 1
-void test_xdp_with_cpumap_helpers(void)
+static void test_xdp_with_cpumap_helpers(void)
{
struct test_xdp_with_cpumap_helpers *skel;
struct bpf_prog_info info = {};
@@ -68,7 +68,7 @@ void test_xdp_with_cpumap_helpers(void)
test_xdp_with_cpumap_helpers__destroy(skel);
}
-void test_xdp_with_cpumap_frags_helpers(void)
+static void test_xdp_with_cpumap_frags_helpers(void)
{
struct test_xdp_with_cpumap_frags_helpers *skel;
struct bpf_prog_info info = {};
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
index 463a72fc3e70..ead40016c324 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c
@@ -81,7 +81,7 @@ static void test_neg_xdp_devmap_helpers(void)
}
}
-void test_xdp_with_devmap_frags_helpers(void)
+static void test_xdp_with_devmap_frags_helpers(void)
{
struct test_xdp_with_devmap_frags_helpers *skel;
struct bpf_prog_info info = {};
--
2.35.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v4 2/2] selftests/bpf: check whether s32 is sufficient for kfunc offset
2022-02-06 4:31 [PATCH bpf-next v4 0/2] selftests: add test for kfunc call Hou Tao
2022-02-06 4:31 ` [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test Hou Tao
@ 2022-02-06 4:31 ` Hou Tao
2022-02-07 18:33 ` Yonghong Song
1 sibling, 1 reply; 7+ messages in thread
From: Hou Tao @ 2022-02-06 4:31 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Yonghong Song,
Andrii Nakryiko, David S . Miller, Jakub Kicinski,
John Fastabend, netdev, bpf, houtao1
In add_kfunc_call(), bpf_kfunc_desc->imm with type s32 is used to
represent the offset of called kfunc from __bpf_call_base, so
add a test to ensure that the offset will not be overflowed.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
.../selftests/bpf/prog_tests/ksyms_module.c | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
index a1ebac70ec29..8055fbbf720b 100644
--- a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
@@ -3,9 +3,49 @@
#include <test_progs.h>
#include <network_helpers.h>
+#include <trace_helpers.h>
#include "test_ksyms_module.lskel.h"
#include "test_ksyms_module.skel.h"
+/*
+ * Check whether or not s32 in bpf_kfunc_desc is sufficient
+ * to represent the offset between bpf_testmod_test_mod_kfunc
+ * and __bpf_call_base.
+ */
+static void test_ksyms_module_valid_offset(void)
+{
+ struct test_ksyms_module *skel;
+ unsigned long long kfunc_addr;
+ unsigned long long base_addr;
+ long long actual_offset;
+ int used_offset;
+ int err;
+
+ if (!env.has_testmod) {
+ test__skip();
+ return;
+ }
+
+ /* Ensure kfunc call is supported */
+ skel = test_ksyms_module__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "test_ksyms_module__open"))
+ return;
+
+ err = kallsyms_find("bpf_testmod_test_mod_kfunc", &kfunc_addr);
+ if (!ASSERT_OK(err, "find kfunc addr"))
+ goto cleanup;
+
+ err = kallsyms_find("__bpf_call_base", &base_addr);
+ if (!ASSERT_OK(err, "find base addr"))
+ goto cleanup;
+
+ used_offset = kfunc_addr - base_addr;
+ actual_offset = kfunc_addr - base_addr;
+ ASSERT_EQ((long long)used_offset, actual_offset, "kfunc offset overflowed");
+cleanup:
+ test_ksyms_module__destroy(skel);
+}
+
static void test_ksyms_module_lskel(void)
{
struct test_ksyms_module_lskel *skel;
@@ -62,6 +102,8 @@ static void test_ksyms_module_libbpf(void)
void test_ksyms_module(void)
{
+ if (test__start_subtest("valid_offset"))
+ test_ksyms_module_valid_offset();
if (test__start_subtest("lskel"))
test_ksyms_module_lskel();
if (test__start_subtest("libbpf"))
--
2.35.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test
2022-02-06 4:31 ` [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test Hou Tao
@ 2022-02-07 18:17 ` Yonghong Song
2022-02-08 1:36 ` Hou Tao
0 siblings, 1 reply; 7+ messages in thread
From: Yonghong Song @ 2022-02-07 18:17 UTC (permalink / raw)
To: Hou Tao, Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Andrii Nakryiko,
David S . Miller, Jakub Kicinski, John Fastabend, netdev, bpf,
houtao1
On 2/5/22 8:31 PM, Hou Tao wrote:
> Two subtests in ksyms_module.c are not qualified as static, so these
> subtests are exported as standalone tests in tests.h and lead to
> confusion for the output of "./test_progs -t ksyms_module".
>
> By using the following command:
>
> grep "^void \(serial_\)\?test_[a-zA-Z0-9_]\+(\(void\)\?)" \
> tools/testing/selftests/bpf/prog_tests/*.c | \
> awk -F : '{print $1}' | sort | uniq -c | awk '$1 != 1'
>
> Find out that other tests also have the similar problem, so fix
> these tests by marking subtests in these tests as static. For
> xdp_adjust_frags.c, there is just one subtest, so just export
> the subtest directly.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> tools/testing/selftests/bpf/prog_tests/ksyms_module.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c | 6 ------
> tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/xdp_cpumap_attach.c | 4 ++--
> tools/testing/selftests/bpf/prog_tests/xdp_devmap_attach.c | 2 +-
> 5 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
> index ecc58c9e7631..a1ebac70ec29 100644
> --- a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
> +++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
> @@ -6,7 +6,7 @@
> #include "test_ksyms_module.lskel.h"
> #include "test_ksyms_module.skel.h"
>
> -void test_ksyms_module_lskel(void)
> +static void test_ksyms_module_lskel(void)
> {
> struct test_ksyms_module_lskel *skel;
> int err;
> @@ -33,7 +33,7 @@ void test_ksyms_module_lskel(void)
> test_ksyms_module_lskel__destroy(skel);
> }
>
> -void test_ksyms_module_libbpf(void)
> +static void test_ksyms_module_libbpf(void)
> {
> struct test_ksyms_module *skel;
> int err;
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> index 134d0ac32f59..fc2d8fa8dac5 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
> @@ -102,9 +102,3 @@ void test_xdp_update_frags(void)
> out:
> bpf_object__close(obj);
> }
> -
> -void test_xdp_adjust_frags(void)
> -{
> - if (test__start_subtest("xdp_adjust_frags"))
> - test_xdp_update_frags();
> -}
I suggest keep test_xdp_adjust_frags and mark
test_xdp_update_frags as static function, and
this is also good for future extension.
It is confusing that test_xdp_update_frags
test in file xdp_adjust_frags.c. Typical
prog_tests/ test has {test,serial_test}_<TEST> test
with file name <TEST>.c file.
[...]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 2/2] selftests/bpf: check whether s32 is sufficient for kfunc offset
2022-02-06 4:31 ` [PATCH bpf-next v4 2/2] selftests/bpf: check whether s32 is sufficient for kfunc offset Hou Tao
@ 2022-02-07 18:33 ` Yonghong Song
2022-02-08 2:47 ` Hou Tao
0 siblings, 1 reply; 7+ messages in thread
From: Yonghong Song @ 2022-02-07 18:33 UTC (permalink / raw)
To: Hou Tao, Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Andrii Nakryiko,
David S . Miller, Jakub Kicinski, John Fastabend, netdev, bpf,
houtao1
On 2/5/22 8:31 PM, Hou Tao wrote:
> In add_kfunc_call(), bpf_kfunc_desc->imm with type s32 is used to
> represent the offset of called kfunc from __bpf_call_base, so
> add a test to ensure that the offset will not be overflowed.
>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
> .../selftests/bpf/prog_tests/ksyms_module.c | 42 +++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
> index a1ebac70ec29..8055fbbf720b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
> +++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
> @@ -3,9 +3,49 @@
>
> #include <test_progs.h>
> #include <network_helpers.h>
> +#include <trace_helpers.h>
> #include "test_ksyms_module.lskel.h"
> #include "test_ksyms_module.skel.h"
>
> +/*
> + * Check whether or not s32 in bpf_kfunc_desc is sufficient
> + * to represent the offset between bpf_testmod_test_mod_kfunc
> + * and __bpf_call_base.
> + */
> +static void test_ksyms_module_valid_offset(void)
> +{
> + struct test_ksyms_module *skel;
> + unsigned long long kfunc_addr;
> + unsigned long long base_addr;
> + long long actual_offset;
> + int used_offset;
> + int err;
> +
> + if (!env.has_testmod) {
> + test__skip();
> + return;
> + }
> +
> + /* Ensure kfunc call is supported */
> + skel = test_ksyms_module__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "test_ksyms_module__open"))
> + return;
> +
> + err = kallsyms_find("bpf_testmod_test_mod_kfunc", &kfunc_addr);
> + if (!ASSERT_OK(err, "find kfunc addr"))
> + goto cleanup;
> +
> + err = kallsyms_find("__bpf_call_base", &base_addr);
> + if (!ASSERT_OK(err, "find base addr"))
> + goto cleanup;
> +
> + used_offset = kfunc_addr - base_addr;
> + actual_offset = kfunc_addr - base_addr;
> + ASSERT_EQ((long long)used_offset, actual_offset, "kfunc offset overflowed");
I am a little bit confused about motivation here. Maybe I missed
something. If we indeed have kfunc offset overflow,
should kernel verifier just reject the program? Specially,
we should make the above test_ksyms_module__open_and_load()
fail?
> +cleanup:
> + test_ksyms_module__destroy(skel);
> +}
> +
> static void test_ksyms_module_lskel(void)
> {
> struct test_ksyms_module_lskel *skel;
> @@ -62,6 +102,8 @@ static void test_ksyms_module_libbpf(void)
>
> void test_ksyms_module(void)
> {
> + if (test__start_subtest("valid_offset"))
> + test_ksyms_module_valid_offset();
> if (test__start_subtest("lskel"))
> test_ksyms_module_lskel();
> if (test__start_subtest("libbpf"))
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test
2022-02-07 18:17 ` Yonghong Song
@ 2022-02-08 1:36 ` Hou Tao
0 siblings, 0 replies; 7+ messages in thread
From: Hou Tao @ 2022-02-08 1:36 UTC (permalink / raw)
To: Yonghong Song, Hou Tao, Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Andrii Nakryiko,
David S . Miller, Jakub Kicinski, John Fastabend, netdev, bpf
Hi,
On 2/8/2022 2:17 AM, Yonghong Song wrote:
>
>
> On 2/5/22 8:31 PM, Hou Tao wrote:
>> Two subtests in ksyms_module.c are not qualified as static, so these
>> subtests are exported as standalone tests in tests.h and lead to
>> confusion for the output of "./test_progs -t ksyms_module".
>>
>> By using the following command:
>>
>> grep "^void \(serial_\)\?test_[a-zA-Z0-9_]\+(\(void\)\?)" \
>> tools/testing/selftests/bpf/prog_tests/*.c | \
>> awk -F : '{print $1}' | sort | uniq -c | awk '$1 != 1'
>>
>> Find out that other tests also have the similar problem, so fix
>> these tests by marking subtests in these tests as static. For
>> xdp_adjust_frags.c, there is just one subtest, so just export
>> the subtest directly.
[...]
>> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> index 134d0ac32f59..fc2d8fa8dac5 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_frags.c
>> @@ -102,9 +102,3 @@ void test_xdp_update_frags(void)
>> out:
>> bpf_object__close(obj);
>> }
>> -
>> -void test_xdp_adjust_frags(void)
>> -{
>> - if (test__start_subtest("xdp_adjust_frags"))
>> - test_xdp_update_frags();
>> -}
>
> I suggest keep test_xdp_adjust_frags and mark
> test_xdp_update_frags as static function, and
> this is also good for future extension.
> It is confusing that test_xdp_update_frags
> test in file xdp_adjust_frags.c. Typical
> prog_tests/ test has {test,serial_test}_<TEST> test
> with file name <TEST>.c file.
>
Will do and thanks for your suggestion.
Regards,
Tao
> [...]
> .
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 2/2] selftests/bpf: check whether s32 is sufficient for kfunc offset
2022-02-07 18:33 ` Yonghong Song
@ 2022-02-08 2:47 ` Hou Tao
0 siblings, 0 replies; 7+ messages in thread
From: Hou Tao @ 2022-02-08 2:47 UTC (permalink / raw)
To: Yonghong Song, Hou Tao, Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Andrii Nakryiko,
David S . Miller, Jakub Kicinski, John Fastabend, netdev, bpf
Hi,
On 2/8/2022 2:33 AM, Yonghong Song wrote:
>
>
> On 2/5/22 8:31 PM, Hou Tao wrote:
>> In add_kfunc_call(), bpf_kfunc_desc->imm with type s32 is used to
>> represent the offset of called kfunc from __bpf_call_base, so
>> add a test to ensure that the offset will not be overflowed.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>> .../selftests/bpf/prog_tests/ksyms_module.c | 42 +++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
[...]
>> + /* Ensure kfunc call is supported */
>> + skel = test_ksyms_module__open_and_load();
>> + if (!ASSERT_OK_PTR(skel, "test_ksyms_module__open"))
>> + return;
>> +
>> + err = kallsyms_find("bpf_testmod_test_mod_kfunc", &kfunc_addr);
>> + if (!ASSERT_OK(err, "find kfunc addr"))
>> + goto cleanup;
>> +
>> + err = kallsyms_find("__bpf_call_base", &base_addr);
>> + if (!ASSERT_OK(err, "find base addr"))
>> + goto cleanup;
>> +
>> + used_offset = kfunc_addr - base_addr;
>> + actual_offset = kfunc_addr - base_addr;
>> + ASSERT_EQ((long long)used_offset, actual_offset, "kfunc offset
>> overflowed");
>
> I am a little bit confused about motivation here. Maybe I missed something. If
> we indeed have kfunc offset overflow,
> should kernel verifier just reject the program? Specially,
> we should make the above test_ksyms_module__open_and_load()
> fail?
In add_kfunc_call(), the calculation of imm doesn't consider the overflow
of s32. So test_ksyms_module__open_and_load() will succeed. I think the
better solution is to put the overflow check in add_kfunc_call(), so will
drop this patch and add the overflow check in add_kfunc_call() instead.
Regards,
Tao
[...]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-08 2:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-06 4:31 [PATCH bpf-next v4 0/2] selftests: add test for kfunc call Hou Tao
2022-02-06 4:31 ` [PATCH bpf-next v4 1/2] selftests/bpf: do not export subtest as standalone test Hou Tao
2022-02-07 18:17 ` Yonghong Song
2022-02-08 1:36 ` Hou Tao
2022-02-06 4:31 ` [PATCH bpf-next v4 2/2] selftests/bpf: check whether s32 is sufficient for kfunc offset Hou Tao
2022-02-07 18:33 ` Yonghong Song
2022-02-08 2:47 ` Hou Tao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.