diff mbox series

[bpf,4/4] selftests/bpf: Add a test case to write into .rodata

Message ID 20240823222033.31006-4-daniel@iogearbox.net (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf,1/4] bpf: Fix helper writes to read-only maps | expand

Checks

Context Check Description
bpf/vmtest-bpf-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-PR fail PR summary
bpf/vmtest-bpf-VM_Test-4 fail Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-6 success Logs for aarch64-gcc / test
bpf/vmtest-bpf-VM_Test-7 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-VM_Test-8 fail Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-VM_Test-9 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-VM_Test-10 success Logs for s390x-gcc / test
bpf/vmtest-bpf-VM_Test-11 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-VM_Test-24 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-VM_Test-14 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-VM_Test-15 success Logs for x86_64-gcc / test
bpf/vmtest-bpf-VM_Test-20 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-VM_Test-18 fail Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-VM_Test-13 fail Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-22 fail Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-VM_Test-16 success Logs for x86_64-gcc / veristat
bpf/vmtest-bpf-VM_Test-21 fail Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-23 success Logs for x86_64-llvm-18 / test
bpf/vmtest-bpf-VM_Test-19 success Logs for x86_64-llvm-17 / test
bpf/vmtest-bpf-VM_Test-17 fail Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-12 success Logs for set-matrix
netdev/series_format warning Series does not have a cover letter
netdev/tree_selection success Clearly marked for bpf, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 14 maintainers not CCed: andrii@kernel.org sdf@fomichev.me eddyz87@gmail.com mykolal@fb.com haoluo@google.com shuah@kernel.org jolsa@kernel.org linux-kselftest@vger.kernel.org song@kernel.org yonghong.song@linux.dev ast@kernel.org kpsingh@kernel.org martin.lau@linux.dev john.fastabend@gmail.com
netdev/build_clang success Errors and warnings before: 17 this patch: 17
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 22 this patch: 22
netdev/checkpatch warning WARNING: Missing a blank line after declarations WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Daniel Borkmann Aug. 23, 2024, 10:20 p.m. UTC 
Add a test case which attempts to write into .rodata section of the
BPF program, and for comparison this adds test cases also for .bss
and .data section.

Before fix:

  # ./vmtest.sh -- ./test_progs -t verifier_const
  [...]
  ./test_progs -t verifier_const
  tester_init:PASS:tester_log_buf 0 nsec
  process_subtest:PASS:obj_open_mem 0 nsec
  process_subtest:PASS:specs_alloc 0 nsec
  run_subtest:PASS:obj_open_mem 0 nsec
  run_subtest:FAIL:unexpected_load_success unexpected success: 0
  #465/1   verifier_const/rodata: write rejected:FAIL
  #465/2   verifier_const/bss: write accepted:OK
  #465/3   verifier_const/data: write accepted:OK
  #465     verifier_const:FAIL
  [...]

After fix:

  # ./vmtest.sh -- ./test_progs -t verifier_const
  [...]
  ./test_progs -t verifier_const
  #465/1   verifier_const/rodata: write rejected:OK
  #465/2   verifier_const/bss: write accepted:OK
  #465/3   verifier_const/data: write accepted:OK
  #465     verifier_const:OK
  [...]

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 .../selftests/bpf/prog_tests/tc_links.c       |  1 +
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../selftests/bpf/progs/verifier_const.c      | 42 +++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_const.c

Comments

Shung-Hsi Yu Aug. 26, 2024, 6:39 a.m. UTC | #1 
On Sat, Aug 24, 2024 at 12:20:33AM GMT, Daniel Borkmann wrote:
> Add a test case which attempts to write into .rodata section of the
> BPF program, and for comparison this adds test cases also for .bss
> and .data section.
> 
> Before fix:
> 
>   # ./vmtest.sh -- ./test_progs -t verifier_const
>   [...]
>   ./test_progs -t verifier_const
>   tester_init:PASS:tester_log_buf 0 nsec
>   process_subtest:PASS:obj_open_mem 0 nsec
>   process_subtest:PASS:specs_alloc 0 nsec
>   run_subtest:PASS:obj_open_mem 0 nsec
>   run_subtest:FAIL:unexpected_load_success unexpected success: 0
>   #465/1   verifier_const/rodata: write rejected:FAIL
>   #465/2   verifier_const/bss: write accepted:OK
>   #465/3   verifier_const/data: write accepted:OK
>   #465     verifier_const:FAIL
>   [...]
> 
> After fix:
> 
>   # ./vmtest.sh -- ./test_progs -t verifier_const
>   [...]
>   ./test_progs -t verifier_const
>   #465/1   verifier_const/rodata: write rejected:OK
>   #465/2   verifier_const/bss: write accepted:OK
>   #465/3   verifier_const/data: write accepted:OK
>   #465     verifier_const:OK
>   [...]
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
[...]

Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Andrii Nakryiko Aug. 27, 2024, 10:39 p.m. UTC | #2 
On Fri, Aug 23, 2024 at 3:20 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Add a test case which attempts to write into .rodata section of the
> BPF program, and for comparison this adds test cases also for .bss
> and .data section.
>
> Before fix:
>
>   # ./vmtest.sh -- ./test_progs -t verifier_const
>   [...]
>   ./test_progs -t verifier_const
>   tester_init:PASS:tester_log_buf 0 nsec
>   process_subtest:PASS:obj_open_mem 0 nsec
>   process_subtest:PASS:specs_alloc 0 nsec
>   run_subtest:PASS:obj_open_mem 0 nsec
>   run_subtest:FAIL:unexpected_load_success unexpected success: 0
>   #465/1   verifier_const/rodata: write rejected:FAIL
>   #465/2   verifier_const/bss: write accepted:OK
>   #465/3   verifier_const/data: write accepted:OK
>   #465     verifier_const:FAIL
>   [...]
>
> After fix:
>
>   # ./vmtest.sh -- ./test_progs -t verifier_const
>   [...]
>   ./test_progs -t verifier_const
>   #465/1   verifier_const/rodata: write rejected:OK
>   #465/2   verifier_const/bss: write accepted:OK
>   #465/3   verifier_const/data: write accepted:OK
>   #465     verifier_const:OK
>   [...]
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  .../selftests/bpf/prog_tests/tc_links.c       |  1 +
>  .../selftests/bpf/prog_tests/verifier.c       |  2 +
>  .../selftests/bpf/progs/verifier_const.c      | 42 +++++++++++++++++++
>  3 files changed, 45 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/verifier_const.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_links.c b/tools/testing/selftests/bpf/prog_tests/tc_links.c
> index 1af9ec1149aa..92c647dfd6f1 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tc_links.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tc_links.c
> @@ -9,6 +9,7 @@
>  #define ping_cmd "ping -q -c1 -w1 127.0.0.1 > /dev/null"
>
>  #include "test_tc_link.skel.h"
> +#include "test_const.skel.h"
>
>  #include "netlink_helpers.h"
>  #include "tc_helpers.h"
> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
> index 9dc3687bc406..c0cb1a145274 100644
> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
> @@ -21,6 +21,7 @@
>  #include "verifier_cgroup_inv_retcode.skel.h"
>  #include "verifier_cgroup_skb.skel.h"
>  #include "verifier_cgroup_storage.skel.h"
> +#include "verifier_const.skel.h"
>  #include "verifier_const_or.skel.h"
>  #include "verifier_ctx.skel.h"
>  #include "verifier_ctx_sk_msg.skel.h"
> @@ -140,6 +141,7 @@ void test_verifier_cfg(void)                  { RUN(verifier_cfg); }
>  void test_verifier_cgroup_inv_retcode(void)   { RUN(verifier_cgroup_inv_retcode); }
>  void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
>  void test_verifier_cgroup_storage(void)       { RUN(verifier_cgroup_storage); }
> +void test_verifier_const(void)                { RUN(verifier_const); }
>  void test_verifier_const_or(void)             { RUN(verifier_const_or); }
>  void test_verifier_ctx(void)                  { RUN(verifier_ctx); }
>  void test_verifier_ctx_sk_msg(void)           { RUN(verifier_ctx_sk_msg); }
> diff --git a/tools/testing/selftests/bpf/progs/verifier_const.c b/tools/testing/selftests/bpf/progs/verifier_const.c
> new file mode 100644
> index 000000000000..81302d9738fa
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_const.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Isovalent */
> +
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +const long foo = 42;

nit: would be "safer" to mark it as "const volatile long" to prevent
whatever smartness compiler might decide to do

> +long bar;
> +long bart = 96;
> +
> +SEC("tc/ingress")
> +__description("rodata: write rejected")
> +__failure __msg("write into map forbidden")
> +int tcx1(struct __sk_buff *skb)
> +{
> +       char buff[] = { '8', '4', '\0' };
> +       bpf_strtol(buff, sizeof(buff), 0, (long *)&foo);
> +       return TCX_PASS;
> +}
> +
> +SEC("tc/ingress")
> +__description("bss: write accepted")
> +__success
> +int tcx2(struct __sk_buff *skb)
> +{
> +       char buff[] = { '8', '4', '\0' };
> +       bpf_strtol(buff, sizeof(buff), 0, &bar);
> +       return TCX_PASS;
> +}
> +
> +SEC("tc/ingress")
> +__description("data: write accepted")
> +__success
> +int tcx3(struct __sk_buff *skb)
> +{
> +       char buff[] = { '8', '4', '\0' };
> +       bpf_strtol(buff, sizeof(buff), 0, &bart);
> +       return TCX_PASS;
> +}
> +
> +char LICENSE[] SEC("license") = "GPL";
> --
> 2.43.0
>
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/tc_links.c b/tools/testing/selftests/bpf/prog_tests/tc_links.c
index 1af9ec1149aa..92c647dfd6f1 100644
--- a/tools/testing/selftests/bpf/prog_tests/tc_links.c
+++ b/tools/testing/selftests/bpf/prog_tests/tc_links.c
@@ -9,6 +9,7 @@ 
 #define ping_cmd "ping -q -c1 -w1 127.0.0.1 > /dev/null"
 
 #include "test_tc_link.skel.h"
+#include "test_const.skel.h"
 
 #include "netlink_helpers.h"
 #include "tc_helpers.h"
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 9dc3687bc406..c0cb1a145274 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -21,6 +21,7 @@ 
 #include "verifier_cgroup_inv_retcode.skel.h"
 #include "verifier_cgroup_skb.skel.h"
 #include "verifier_cgroup_storage.skel.h"
+#include "verifier_const.skel.h"
 #include "verifier_const_or.skel.h"
 #include "verifier_ctx.skel.h"
 #include "verifier_ctx_sk_msg.skel.h"
@@ -140,6 +141,7 @@  void test_verifier_cfg(void)                  { RUN(verifier_cfg); }
 void test_verifier_cgroup_inv_retcode(void)   { RUN(verifier_cgroup_inv_retcode); }
 void test_verifier_cgroup_skb(void)           { RUN(verifier_cgroup_skb); }
 void test_verifier_cgroup_storage(void)       { RUN(verifier_cgroup_storage); }
+void test_verifier_const(void)                { RUN(verifier_const); }
 void test_verifier_const_or(void)             { RUN(verifier_const_or); }
 void test_verifier_ctx(void)                  { RUN(verifier_ctx); }
 void test_verifier_ctx_sk_msg(void)           { RUN(verifier_ctx_sk_msg); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_const.c b/tools/testing/selftests/bpf/progs/verifier_const.c
new file mode 100644
index 000000000000..81302d9738fa
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_const.c
@@ -0,0 +1,42 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Isovalent */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+const long foo = 42;
+long bar;
+long bart = 96;
+
+SEC("tc/ingress")
+__description("rodata: write rejected")
+__failure __msg("write into map forbidden")
+int tcx1(struct __sk_buff *skb)
+{
+	char buff[] = { '8', '4', '\0' };
+	bpf_strtol(buff, sizeof(buff), 0, (long *)&foo);
+	return TCX_PASS;
+}
+
+SEC("tc/ingress")
+__description("bss: write accepted")
+__success
+int tcx2(struct __sk_buff *skb)
+{
+	char buff[] = { '8', '4', '\0' };
+	bpf_strtol(buff, sizeof(buff), 0, &bar);
+	return TCX_PASS;
+}
+
+SEC("tc/ingress")
+__description("data: write accepted")
+__success
+int tcx3(struct __sk_buff *skb)
+{
+	char buff[] = { '8', '4', '\0' };
+	bpf_strtol(buff, sizeof(buff), 0, &bart);
+	return TCX_PASS;
+}
+
+char LICENSE[] SEC("license") = "GPL";