[RFC PATCH bpf-next 0/7] BPF static linker: global symbols visibility

[RFC PATCH bpf-next 0/7] BPF static linker: global symbols visibility

[Andrii Nakryiko]

This RFC explores dropping static variables from BPF skeleton and leaving them
for use only within BPF object file. Instead, BPF static linker is extended
with support for controlling global symbol visibility outside of a single BPF
object file through STV_HIDDEN and STV_INTERNAL ELF symbol visibility.

See patch #7 for all the details, justification, and comparison with
user-space linker behavior.

Andrii Nakryiko (7):
  bpftool: strip const/volatile/restrict modifiers from .bss and .data
    vars
  libbpf: add per-file linker opts
  selftests/bpf: stop using static variables for passing data to/from
    user-space
  bpftool: stop emitting static variables in BPF skeleton
  libbpf: fix ELF symbol visibility update logic
  libbpf: treat STV_INTERNAL same as STV_HIDDEN for functions
  libbpf: convert STV_HIDDEN symbols into STV_INTERNAL after linking

 tools/bpf/bpftool/gen.c                       |  8 ++-
 tools/lib/bpf/libbpf.c                        | 11 ++--
 tools/lib/bpf/libbpf.h                        | 10 ++-
 tools/lib/bpf/linker.c                        | 62 +++++++++++++++----
 .../selftests/bpf/prog_tests/send_signal.c    |  2 +-
 .../selftests/bpf/prog_tests/skeleton.c       |  6 +-
 .../selftests/bpf/prog_tests/static_linked.c  |  5 --
 .../selftests/bpf/progs/bpf_iter_test_kern4.c |  4 +-
 tools/testing/selftests/bpf/progs/kfree_skb.c |  4 +-
 tools/testing/selftests/bpf/progs/tailcall3.c |  2 +-
 tools/testing/selftests/bpf/progs/tailcall4.c |  2 +-
 tools/testing/selftests/bpf/progs/tailcall5.c |  2 +-
 .../selftests/bpf/progs/tailcall_bpf2bpf2.c   |  2 +-
 .../selftests/bpf/progs/tailcall_bpf2bpf4.c   |  2 +-
 .../selftests/bpf/progs/test_check_mtu.c      |  4 +-
 .../selftests/bpf/progs/test_cls_redirect.c   |  4 +-
 .../bpf/progs/test_global_func_args.c         |  2 +-
 .../selftests/bpf/progs/test_rdonly_maps.c    |  6 +-
 .../selftests/bpf/progs/test_skeleton.c       |  4 +-
 .../bpf/progs/test_snprintf_single.c          |  2 +-
 .../selftests/bpf/progs/test_sockmap_listen.c |  4 +-
 .../selftests/bpf/progs/test_static_linked1.c |  8 +--
 .../selftests/bpf/progs/test_static_linked2.c |  8 +--
 23 files changed, 105 insertions(+), 59 deletions(-)

-- 
2.30.2

[RFC PATCH bpf-next 1/7] bpftool: strip const/volatile/restrict modifiers from .bss and .data vars

[Andrii Nakryiko]

Similarly to .rodata, strip any const/volatile/restrict modifiers when
generating BPF skeleton. They are not helpful and actually just get in the way.

Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/bpf/bpftool/gen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 31ade77f5ef8..440a2fcb6441 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -106,8 +106,10 @@ static int codegen_datasec_def(struct bpf_object *obj,
 
 	if (strcmp(sec_name, ".data") == 0) {
 		sec_ident = "data";
+		strip_mods = true;
 	} else if (strcmp(sec_name, ".bss") == 0) {
 		sec_ident = "bss";
+		strip_mods = true;
 	} else if (strcmp(sec_name, ".rodata") == 0) {
 		sec_ident = "rodata";
 		strip_mods = true;
-- 
2.30.2

[RFC PATCH bpf-next 2/7] libbpf: add per-file linker opts

[Andrii Nakryiko]

For better future extensibility add per-file linker options. Currently
the set of available options is empty. This changes bpf_linker__add_file()
API, but it's not a breaking change as bpf_linker APIs hasn't been released
yet.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/bpf/bpftool/gen.c |  2 +-
 tools/lib/bpf/libbpf.h  | 10 +++++++++-
 tools/lib/bpf/linker.c  | 16 ++++++++++++----
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 440a2fcb6441..06fee4a2910a 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -638,7 +638,7 @@ static int do_object(int argc, char **argv)
 	while (argc) {
 		file = GET_ARG();
 
-		err = bpf_linker__add_file(linker, file);
+		err = bpf_linker__add_file(linker, file, NULL);
 		if (err) {
 			p_err("failed to link '%s': %s (%d)", file, strerror(err), err);
 			goto out;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index bec4e6a6e31d..3f3a24763459 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -768,10 +768,18 @@ struct bpf_linker_opts {
 };
 #define bpf_linker_opts__last_field sz
 
+struct bpf_linker_file_opts {
+	/* size of this struct, for forward/backward compatiblity */
+	size_t sz;
+};
+#define bpf_linker_file_opts__last_field sz
+
 struct bpf_linker;
 
 LIBBPF_API struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts *opts);
-LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker, const char *filename);
+LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker,
+				    const char *filename,
+				    const struct bpf_linker_file_opts *opts);
 LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
 LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
 
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index 9de084b1c699..3b1fbc27be37 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -158,7 +158,9 @@ struct bpf_linker {
 
 static int init_output_elf(struct bpf_linker *linker, const char *file);
 
-static int linker_load_obj_file(struct bpf_linker *linker, const char *filename, struct src_obj *obj);
+static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
+				const struct bpf_linker_file_opts *opts,
+				struct src_obj *obj);
 static int linker_sanity_check_elf(struct src_obj *obj);
 static int linker_sanity_check_elf_symtab(struct src_obj *obj, struct src_sec *sec);
 static int linker_sanity_check_elf_relos(struct src_obj *obj, struct src_sec *sec);
@@ -435,15 +437,19 @@ static int init_output_elf(struct bpf_linker *linker, const char *file)
 	return 0;
 }
 
-int bpf_linker__add_file(struct bpf_linker *linker, const char *filename)
+int bpf_linker__add_file(struct bpf_linker *linker, const char *filename,
+			 const struct bpf_linker_file_opts *opts)
 {
 	struct src_obj obj = {};
 	int err = 0;
 
+	if (!OPTS_VALID(opts, bpf_linker_file_opts))
+		return -EINVAL;
+
 	if (!linker->elf)
 		return -EINVAL;
 
-	err = err ?: linker_load_obj_file(linker, filename, &obj);
+	err = err ?: linker_load_obj_file(linker, filename, opts, &obj);
 	err = err ?: linker_append_sec_data(linker, &obj);
 	err = err ?: linker_append_elf_syms(linker, &obj);
 	err = err ?: linker_append_elf_relos(linker, &obj);
@@ -529,7 +535,9 @@ static struct src_sec *add_src_sec(struct src_obj *obj, const char *sec_name)
 	return sec;
 }
 
-static int linker_load_obj_file(struct bpf_linker *linker, const char *filename, struct src_obj *obj)
+static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
+				const struct bpf_linker_file_opts *opts,
+				struct src_obj *obj)
 {
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 	const int host_endianness = ELFDATA2LSB;
-- 
2.30.2

[RFC PATCH bpf-next 3/7] selftests/bpf: stop using static variables for passing data to/from user-space

[Andrii Nakryiko]

In preparation of skipping emitting static variables in BPF skeletons, switch
all current selftests uses of static variables to pass data between BPF and
user-space to use global variables.

All non-read-only `static volatile` variables become just plain global
variables by dropping `static volatile` part.

Read-only `static volatile const` variables, though, still require `volatile`
modifier, otherwise compiler will ignore whatever values are set from
user-space.

Few static linker tests are using name-conflicting static variables to
validate that static linker still properly handles static variables and
doesn't trip up on name conflicts.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/send_signal.c      | 2 +-
 tools/testing/selftests/bpf/prog_tests/skeleton.c         | 6 ++----
 tools/testing/selftests/bpf/prog_tests/static_linked.c    | 5 -----
 tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c   | 4 ++--
 tools/testing/selftests/bpf/progs/kfree_skb.c             | 4 ++--
 tools/testing/selftests/bpf/progs/tailcall3.c             | 2 +-
 tools/testing/selftests/bpf/progs/tailcall4.c             | 2 +-
 tools/testing/selftests/bpf/progs/tailcall5.c             | 2 +-
 tools/testing/selftests/bpf/progs/tailcall_bpf2bpf2.c     | 2 +-
 tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c     | 2 +-
 tools/testing/selftests/bpf/progs/test_check_mtu.c        | 4 ++--
 tools/testing/selftests/bpf/progs/test_cls_redirect.c     | 4 ++--
 tools/testing/selftests/bpf/progs/test_global_func_args.c | 2 +-
 tools/testing/selftests/bpf/progs/test_rdonly_maps.c      | 6 +++---
 tools/testing/selftests/bpf/progs/test_skeleton.c         | 4 ++--
 tools/testing/selftests/bpf/progs/test_snprintf_single.c  | 2 +-
 tools/testing/selftests/bpf/progs/test_sockmap_listen.c   | 4 ++--
 tools/testing/selftests/bpf/progs/test_static_linked1.c   | 8 ++++----
 tools/testing/selftests/bpf/progs/test_static_linked2.c   | 8 ++++----
 19 files changed, 33 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 7043e6ded0e6..a1eade51d440 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -2,7 +2,7 @@
 #include <test_progs.h>
 #include "test_send_signal_kern.skel.h"
 
-static volatile int sigusr1_received = 0;
+int sigusr1_received = 0;
 
 static void sigusr1_handler(int signum)
 {
diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c
index fe87b77af459..f6f130c99b8c 100644
--- a/tools/testing/selftests/bpf/prog_tests/skeleton.c
+++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c
@@ -82,10 +82,8 @@ void test_skeleton(void)
 	CHECK(data->out2 != 2, "res2", "got %lld != exp %d\n", data->out2, 2);
 	CHECK(bss->out3 != 3, "res3", "got %d != exp %d\n", (int)bss->out3, 3);
 	CHECK(bss->out4 != 4, "res4", "got %lld != exp %d\n", bss->out4, 4);
-	CHECK(bss->handler_out5.a != 5, "res5", "got %d != exp %d\n",
-	      bss->handler_out5.a, 5);
-	CHECK(bss->handler_out5.b != 6, "res6", "got %lld != exp %d\n",
-	      bss->handler_out5.b, 6);
+	CHECK(bss->out5.a != 5, "res5", "got %d != exp %d\n", bss->out5.a, 5);
+	CHECK(bss->out5.b != 6, "res6", "got %lld != exp %d\n", bss->out5.b, 6);
 	CHECK(bss->out6 != 14, "res7", "got %d != exp %d\n", bss->out6, 14);
 
 	CHECK(bss->bpf_syscall != kcfg->CONFIG_BPF_SYSCALL, "ext1",
diff --git a/tools/testing/selftests/bpf/prog_tests/static_linked.c b/tools/testing/selftests/bpf/prog_tests/static_linked.c
index 46556976dccc..ab6acbaf9d8c 100644
--- a/tools/testing/selftests/bpf/prog_tests/static_linked.c
+++ b/tools/testing/selftests/bpf/prog_tests/static_linked.c
@@ -14,12 +14,7 @@ void test_static_linked(void)
 		return;
 
 	skel->rodata->rovar1 = 1;
-	skel->bss->static_var1 = 2;
-	skel->bss->static_var11 = 3;
-
 	skel->rodata->rovar2 = 4;
-	skel->bss->static_var2 = 5;
-	skel->bss->static_var22 = 6;
 
 	err = test_static_linked__load(skel);
 	if (!ASSERT_OK(err, "skel_load"))
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
index ee49493dc125..400fdf8d6233 100644
--- a/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
+++ b/tools/testing/selftests/bpf/progs/bpf_iter_test_kern4.c
@@ -9,8 +9,8 @@ __u32 map1_id = 0, map2_id = 0;
 __u32 map1_accessed = 0, map2_accessed = 0;
 __u64 map1_seqnum = 0, map2_seqnum1 = 0, map2_seqnum2 = 0;
 
-static volatile const __u32 print_len;
-static volatile const __u32 ret1;
+volatile const __u32 print_len;
+volatile const __u32 ret1;
 
 SEC("iter/bpf_map")
 int dump_bpf_map(struct bpf_iter__bpf_map *ctx)
diff --git a/tools/testing/selftests/bpf/progs/kfree_skb.c b/tools/testing/selftests/bpf/progs/kfree_skb.c
index a46a264ce24e..55e283050cab 100644
--- a/tools/testing/selftests/bpf/progs/kfree_skb.c
+++ b/tools/testing/selftests/bpf/progs/kfree_skb.c
@@ -109,10 +109,10 @@ int BPF_PROG(trace_kfree_skb, struct sk_buff *skb, void *location)
 	return 0;
 }
 
-static volatile struct {
+struct {
 	bool fentry_test_ok;
 	bool fexit_test_ok;
-} result;
+} result = {};
 
 SEC("fentry/eth_type_trans")
 int BPF_PROG(fentry_eth_type_trans, struct sk_buff *skb, struct net_device *dev,
diff --git a/tools/testing/selftests/bpf/progs/tailcall3.c b/tools/testing/selftests/bpf/progs/tailcall3.c
index 739dc2a51e74..910858fe078a 100644
--- a/tools/testing/selftests/bpf/progs/tailcall3.c
+++ b/tools/testing/selftests/bpf/progs/tailcall3.c
@@ -10,7 +10,7 @@ struct {
 	__uint(value_size, sizeof(__u32));
 } jmp_table SEC(".maps");
 
-static volatile int count;
+int count = 0;
 
 SEC("classifier/0")
 int bpf_func_0(struct __sk_buff *skb)
diff --git a/tools/testing/selftests/bpf/progs/tailcall4.c b/tools/testing/selftests/bpf/progs/tailcall4.c
index f82075b47d7d..bd4be135c39d 100644
--- a/tools/testing/selftests/bpf/progs/tailcall4.c
+++ b/tools/testing/selftests/bpf/progs/tailcall4.c
@@ -10,7 +10,7 @@ struct {
 	__uint(value_size, sizeof(__u32));
 } jmp_table SEC(".maps");
 
-static volatile int selector;
+int selector = 0;
 
 #define TAIL_FUNC(x)				\
 	SEC("classifier/" #x)			\
diff --git a/tools/testing/selftests/bpf/progs/tailcall5.c b/tools/testing/selftests/bpf/progs/tailcall5.c
index ce5450744fd4..adf30a33064e 100644
--- a/tools/testing/selftests/bpf/progs/tailcall5.c
+++ b/tools/testing/selftests/bpf/progs/tailcall5.c
@@ -10,7 +10,7 @@ struct {
 	__uint(value_size, sizeof(__u32));
 } jmp_table SEC(".maps");
 
-static volatile int selector;
+int selector = 0;
 
 #define TAIL_FUNC(x)				\
 	SEC("classifier/" #x)			\
diff --git a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf2.c b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf2.c
index 7b1c04183824..3cc4c12817b5 100644
--- a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf2.c
+++ b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf2.c
@@ -20,7 +20,7 @@ int subprog_tail(struct __sk_buff *skb)
 	return 1;
 }
 
-static volatile int count;
+int count = 0;
 
 SEC("classifier/0")
 int bpf_func_0(struct __sk_buff *skb)
diff --git a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c
index 9a1b166b7fbe..77df6d4db895 100644
--- a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c
+++ b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf4.c
@@ -9,7 +9,7 @@ struct {
 	__uint(value_size, sizeof(__u32));
 } jmp_table SEC(".maps");
 
-static volatile int count;
+int count = 0;
 
 __noinline
 int subprog_tail_2(struct __sk_buff *skb)
diff --git a/tools/testing/selftests/bpf/progs/test_check_mtu.c b/tools/testing/selftests/bpf/progs/test_check_mtu.c
index c4a9bae96e75..71184af57749 100644
--- a/tools/testing/selftests/bpf/progs/test_check_mtu.c
+++ b/tools/testing/selftests/bpf/progs/test_check_mtu.c
@@ -11,8 +11,8 @@
 char _license[] SEC("license") = "GPL";
 
 /* Userspace will update with MTU it can see on device */
-static volatile const int GLOBAL_USER_MTU;
-static volatile const __u32 GLOBAL_USER_IFINDEX;
+volatile const int GLOBAL_USER_MTU;
+volatile const __u32 GLOBAL_USER_IFINDEX;
 
 /* BPF-prog will update these with MTU values it can see */
 __u32 global_bpf_mtu_xdp = 0;
diff --git a/tools/testing/selftests/bpf/progs/test_cls_redirect.c b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
index 3c1e042962e6..e2a5acc4785c 100644
--- a/tools/testing/selftests/bpf/progs/test_cls_redirect.c
+++ b/tools/testing/selftests/bpf/progs/test_cls_redirect.c
@@ -39,8 +39,8 @@ char _license[] SEC("license") = "Dual BSD/GPL";
 /**
  * Destination port and IP used for UDP encapsulation.
  */
-static volatile const __be16 ENCAPSULATION_PORT;
-static volatile const __be32 ENCAPSULATION_IP;
+volatile const __be16 ENCAPSULATION_PORT;
+volatile const __be32 ENCAPSULATION_IP;
 
 typedef struct {
 	uint64_t processed_packets_total;
diff --git a/tools/testing/selftests/bpf/progs/test_global_func_args.c b/tools/testing/selftests/bpf/progs/test_global_func_args.c
index cae309538a9e..e712bf77daae 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func_args.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func_args.c
@@ -8,7 +8,7 @@ struct S {
 	int v;
 };
 
-static volatile struct S global_variable;
+struct S global_variable = {};
 
 struct {
 	__uint(type, BPF_MAP_TYPE_ARRAY);
diff --git a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
index ecbeea2df259..fc8e8a34a3db 100644
--- a/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
+++ b/tools/testing/selftests/bpf/progs/test_rdonly_maps.c
@@ -5,7 +5,7 @@
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 
-static volatile const struct {
+const struct {
 	unsigned a[4];
 	/*
 	 * if the struct's size is multiple of 16, compiler will put it into
@@ -15,11 +15,11 @@ static volatile const struct {
 	char _y;
 } rdonly_values = { .a = {2, 3, 4, 5} };
 
-static volatile struct {
+struct {
 	unsigned did_run;
 	unsigned iters;
 	unsigned sum;
-} res;
+} res = {};
 
 SEC("raw_tracepoint/sys_enter:skip_loop")
 int skip_loop(struct pt_regs *ctx)
diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c
index 374ccef704e1..441fa1c552c8 100644
--- a/tools/testing/selftests/bpf/progs/test_skeleton.c
+++ b/tools/testing/selftests/bpf/progs/test_skeleton.c
@@ -38,11 +38,11 @@ extern int LINUX_KERNEL_VERSION __kconfig;
 bool bpf_syscall = 0;
 int kern_ver = 0;
 
+struct s out5 = {};
+
 SEC("raw_tp/sys_enter")
 int handler(const void *ctx)
 {
-	static volatile struct s out5;
-
 	out1 = in1;
 	out2 = in2;
 	out3 = in3;
diff --git a/tools/testing/selftests/bpf/progs/test_snprintf_single.c b/tools/testing/selftests/bpf/progs/test_snprintf_single.c
index 402adaf344f9..3095837334d3 100644
--- a/tools/testing/selftests/bpf/progs/test_snprintf_single.c
+++ b/tools/testing/selftests/bpf/progs/test_snprintf_single.c
@@ -5,7 +5,7 @@
 #include <bpf/bpf_helpers.h>
 
 /* The format string is filled from the userspace such that loading fails */
-static const char fmt[10];
+const char fmt[10];
 
 SEC("raw_tp/sys_enter")
 int handler(const void *ctx)
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_listen.c b/tools/testing/selftests/bpf/progs/test_sockmap_listen.c
index a39eba9f5201..a1cc58b10c7c 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_listen.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_listen.c
@@ -28,8 +28,8 @@ struct {
 	__type(value, unsigned int);
 } verdict_map SEC(".maps");
 
-static volatile bool test_sockmap; /* toggled by user-space */
-static volatile bool test_ingress; /* toggled by user-space */
+bool test_sockmap = false; /* toggled by user-space */
+bool test_ingress = false; /* toggled by user-space */
 
 SEC("sk_skb/stream_parser")
 int prog_stream_parser(struct __sk_buff *skb)
diff --git a/tools/testing/selftests/bpf/progs/test_static_linked1.c b/tools/testing/selftests/bpf/progs/test_static_linked1.c
index ea1a6c4c7172..cae304045d9c 100644
--- a/tools/testing/selftests/bpf/progs/test_static_linked1.c
+++ b/tools/testing/selftests/bpf/progs/test_static_linked1.c
@@ -4,9 +4,9 @@
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 
-/* 8-byte aligned .bss */
-static volatile long static_var1;
-static volatile int static_var11;
+/* 8-byte aligned .data */
+static volatile long static_var1 = 2;
+static volatile int static_var2 = 3;
 int var1 = 0;
 /* 4-byte aligned .rodata */
 const volatile int rovar1;
@@ -21,7 +21,7 @@ static __noinline int subprog(int x)
 SEC("raw_tp/sys_enter")
 int handler1(const void *ctx)
 {
-	var1 = subprog(rovar1) + static_var1 + static_var11;
+	var1 = subprog(rovar1) + static_var1 + static_var2;
 
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/test_static_linked2.c b/tools/testing/selftests/bpf/progs/test_static_linked2.c
index 54d8d1ab577c..c54c4e865ed8 100644
--- a/tools/testing/selftests/bpf/progs/test_static_linked2.c
+++ b/tools/testing/selftests/bpf/progs/test_static_linked2.c
@@ -4,9 +4,9 @@
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 
-/* 4-byte aligned .bss */
-static volatile int static_var2;
-static volatile int static_var22;
+/* 4-byte aligned .data */
+static volatile int static_var1 = 5;
+static volatile int static_var2 = 6;
 int var2 = 0;
 /* 8-byte aligned .rodata */
 const volatile long rovar2;
@@ -21,7 +21,7 @@ static __noinline int subprog(int x)
 SEC("raw_tp/sys_enter")
 int handler2(const void *ctx)
 {
-	var2 = subprog(rovar2) + static_var2 + static_var22;
+	var2 = subprog(rovar2) + static_var1 + static_var2;
 
 	return 0;
 }
-- 
2.30.2

[RFC PATCH bpf-next 4/7] bpftool: stop emitting static variables in BPF skeleton

[Andrii Nakryiko]

As discussed in [0], stop emitting static variables in BPF skeletons to avoid
issues with name-conflicting static variables across multiple
statically-linked BPF object files.

Users using static variables to pass data between BPF programs and user-space
should do a trivial one-time switch according to the following simple rules:
  - read-only `static volatile const` variables should be converted to
    `volatile const`;
  - read/write `static volatile` variables should just drop `static volatile`
    modifiers to become global variables/symbols. To better handle older Clang
    versions, such newly converted global variables should be explicitly
    initialized with a specific value or `= 0`/`= {}`, whichever is
    appropriate.

  [0] https://lore.kernel.org/bpf/CAEf4BzZo7_r-hsNvJt3w3kyrmmBJj7ghGY8+k4nvKF0KLjma=w@mail.gmail.com/T/#m664d4b0d6b31ac8b2669360e0fc2d6962e9f5ec1

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/bpf/bpftool/gen.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 06fee4a2910a..27dceaf66ecb 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -131,6 +131,10 @@ static int codegen_datasec_def(struct bpf_object *obj,
 		int need_off = sec_var->offset, align_off, align;
 		__u32 var_type_id = var->type;
 
+		/* static variables are not exposed through BPF skeleton */
+		if (btf_var(var)->linkage == BTF_VAR_STATIC)
+			continue;
+
 		if (off > need_off) {
 			p_err("Something is wrong for %s's variable #%d: need offset %d, already at %d.\n",
 			      sec_name, i, need_off, off);
-- 
2.30.2

[RFC PATCH bpf-next 5/7] libbpf: fix ELF symbol visibility update logic

[Andrii Nakryiko]

Fix silly bug in updating ELF symbol's visibility.

Fixes: a46349227cd8 ("libbpf: Add linker extern resolution support for functions and global variables")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/linker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index 3b1fbc27be37..b594a88620ce 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -1788,7 +1788,7 @@ static void sym_update_visibility(Elf64_Sym *sym, int sym_vis)
 	/* libelf doesn't provide setters for ST_VISIBILITY,
 	 * but it is stored in the lower 2 bits of st_other
 	 */
-	sym->st_other &= 0x03;
+	sym->st_other &= ~0x03;
 	sym->st_other |= sym_vis;
 }
 
-- 
2.30.2

[RFC PATCH bpf-next 6/7] libbpf: treat STV_INTERNAL same as STV_HIDDEN for functions

[Andrii Nakryiko]

Do the same global -> static BTF update for global functions with STV_INTERNAL
visibility to turn on static BPF verification mode.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e2a3cf437814..b8cf93fa1b4d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -700,13 +700,14 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data,
 		if (err)
 			return err;
 
-		/* if function is a global/weak symbol, but has hidden
-		 * visibility (STV_HIDDEN), mark its BTF FUNC as static to
-		 * enable more permissive BPF verification mode with more
-		 * outside context available to BPF verifier
+		/* if function is a global/weak symbol, but has restricted
+		 * (STV_HIDDEN or STV_INTERNAL) visibility, mark its BTF FUNC
+		 * as static to enable more permissive BPF verification mode
+		 * with more outside context available to BPF verifier
 		 */
 		if (GELF_ST_BIND(sym.st_info) != STB_LOCAL
-		    && GELF_ST_VISIBILITY(sym.st_other) == STV_HIDDEN)
+		    && (GELF_ST_VISIBILITY(sym.st_other) == STV_HIDDEN
+			|| GELF_ST_VISIBILITY(sym.st_other) == STV_INTERNAL))
 			prog->mark_btf_static = true;
 
 		nr_progs++;
-- 
2.30.2

[RFC PATCH bpf-next 7/7] libbpf: convert STV_HIDDEN symbols into STV_INTERNAL after linking

[Andrii Nakryiko]

Add logic to "upgrade" STV_HIDDEN symbols into STV_INTERNAL ones once static
linking is complete. From BPF static linker perspective, the difference
between STV_HIDDEN and STV_INTERNAL is that STV_HIDDEN allows resolving
externs references between multiple BPF object files statically linked
together in the same operation. STV_INTERNAL is stricter and prevents any
other object files to reference such symbol (function, variable, map).

Here's why such property and behavior is important and desirable and how it
relates to user-space linker behavior.

In user-space, STV_HIDDEN symbols are resolvable during static linking between
all the object files linked together. If such object files are linked into
a shared library (.so), linker is converting such hidden global variables into
STB_LOCAL (static) ones. This has two implications, one of which is desirable
for BPF static linking and one is not.

The desirable one is that such hidden symbols are effectively unresolvable
outside of the shared library. This is exactly the property we are looking for
for BPF libraries (ignoring the fact that we have static BPF libraries, not
dynamic ones).

The undesirable behavior is that after shared library is linked, application
can define "conflicting" symbol with the same name and they will happily
co-exists, because shared library's symbol is now STB_LOCAL. This patch set's
assumption is that static variables are not exposed to BPF skeleton and static
maps are explicitly not supported by libbpf. Both for reasons of avoiding
naming conflicts.

So, if we were to follow user-space behavior exactly, we'd end up with static
maps and variables, which are invisible to BPF skeleton and (for maps) looking
up by name becomes ambiguous.

To avoid such issues, it was decided to convert STV_HIDDEN symbols to
STV_INTERNAL with the semantics that prevents any extern references to
STV_INTERNAL symbols. STV_INTERNAL is not defined for user-space applications
and is reserved for future uses. So in this case BPF static linker is
defining its own semantics.

Having STV_INTERNAL visibility is useful/important for one more reason. With
BPF skeleton not supporting static variables, with just STV_HIDDEN it's
impossible to declare a global variable that will be accessible from
user-space, but not resolvable from other BPF object files. STV_INTERNAL allow
to specify just such behavior:

__attribute__((visibility("internal"))) int user_space_var;

user_space_var is global, but no `extern int user_space_var;` is allowed. Yet,
user-space will be able to read/write such variable with BPF skeleton.

The initial idea was to use only STV_HIDDEN to restrict symbol resolution
outside of BPF library and treat it as STV_INTERNAL described above (i.e., no
other files can extern such symbol). But unfortunately this works only for
single-file BPF libraries, which is quite limiting. Making STV_HIDDEN so
restrictive also prevents having global functions, resolvable withing BPF
library, but treated as static by BPF verification logic. STV_HIDDEN and
STV_INTERNAL resolves all such discrepancies.

Note that currently Clang generates STV_PROTECTED instead of STV_INTERNAL for
__attribute__((visibility("internal"))) for BPF target. Once this bug is
fixed, I'll add new selftests with extra validations.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/linker.c | 44 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index b594a88620ce..38e9ac3dfa8c 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -175,6 +175,7 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
 static int linker_append_btf(struct bpf_linker *linker, struct src_obj *obj);
 static int linker_append_btf_ext(struct bpf_linker *linker, struct src_obj *obj);
 
+static void finalize_elf_syms(struct bpf_linker *linker);
 static int finalize_btf(struct bpf_linker *linker);
 static int finalize_btf_ext(struct bpf_linker *linker);
 
@@ -809,7 +810,7 @@ static int linker_sanity_check_elf_symtab(struct src_obj *obj, struct src_sec *s
 				i, sec->sec_idx, sym_bind);
 			return -EINVAL;
 		}
-		if (sym_vis != STV_DEFAULT && sym_vis != STV_HIDDEN) {
+		if (sym_vis != STV_DEFAULT && sym_vis != STV_HIDDEN && sym_vis != STV_INTERNAL) {
 			pr_warn("ELF sym #%d in section #%zu has unsupported symbol visibility %d\n",
 				i, sec->sec_idx, sym_vis);
 			return -EINVAL;
@@ -1783,6 +1784,15 @@ static void sym_update_type(Elf64_Sym *sym, int sym_type)
 	sym->st_info = ELF64_ST_INFO(ELF64_ST_BIND(sym->st_info), sym_type);
 }
 
+static bool sym_visibility_is_stricter(int src_vis, int dst_vis)
+{
+	if (src_vis == STV_INTERNAL)
+		return dst_vis != STV_INTERNAL;
+	if (src_vis == STV_HIDDEN)
+		return dst_vis == STV_DEFAULT;
+	return false;
+}
+
 static void sym_update_visibility(Elf64_Sym *sym, int sym_vis)
 {
 	/* libelf doesn't provide setters for ST_VISIBILITY,
@@ -1798,7 +1808,7 @@ static int linker_append_elf_sym(struct bpf_linker *linker, struct src_obj *obj,
 	struct src_sec *src_sec = NULL;
 	struct dst_sec *dst_sec = NULL;
 	struct glob_sym *glob_sym = NULL;
-	int name_off, sym_type, sym_bind, sym_vis, err;
+	int name_off, sym_type, sym_bind, sym_vis, dst_vis, err;
 	int btf_sec_id = 0, btf_id = 0;
 	size_t dst_sym_idx;
 	Elf64_Sym *dst_sym;
@@ -1877,10 +1887,16 @@ static int linker_append_elf_sym(struct bpf_linker *linker, struct src_obj *obj,
 			return -EINVAL;
 		}
 
-		if (!glob_syms_match(sym_name, linker, glob_sym, obj, sym, src_sym_idx, btf_id))
+		dst_sym = get_sym_by_idx(linker, glob_sym->sym_idx);
+		dst_vis = ELF64_ST_VISIBILITY(dst_sym->st_other);
+		if (sym_vis == STV_INTERNAL || dst_vis == STV_INTERNAL) {
+			pr_warn("conflicting non-resolvable symbol #%d (%s) definition in '%s'\n",
+				src_sym_idx, sym_name, obj->filename);
 			return -EINVAL;
+		}
 
-		dst_sym = get_sym_by_idx(linker, glob_sym->sym_idx);
+		if (!glob_syms_match(sym_name, linker, glob_sym, obj, sym, src_sym_idx, btf_id))
+			return -EINVAL;
 
 		/* If new symbol is strong, then force dst_sym to be strong as
 		 * well; this way a mix of weak and non-weak extern
@@ -1898,10 +1914,10 @@ static int linker_append_elf_sym(struct bpf_linker *linker, struct src_obj *obj,
 		/* Non-default visibility is "contaminating", with stricter
 		 * visibility overwriting more permissive ones, even if more
 		 * permissive visibility comes from just an extern definition.
-		 * Currently only STV_DEFAULT and STV_HIDDEN are allowed and
-		 * ensured by ELF symbol sanity checks above.
+		 * Currently only STV_DEFAULT, STV_HIDDEN, and STV_INTERNAL
+		 * are allowed and ensured by ELF symbol sanity checks above.
 		 */
-		if (sym_vis > ELF64_ST_VISIBILITY(dst_sym->st_other))
+		if (sym_visibility_is_stricter(sym_vis, ELF64_ST_VISIBILITY(dst_sym->st_other)))
 			sym_update_visibility(dst_sym, sym_vis);
 
 		/* If the new symbol is extern, then regardless if
@@ -2549,6 +2565,8 @@ int bpf_linker__finalize(struct bpf_linker *linker)
 	if (!linker->elf)
 		return -EINVAL;
 
+	finalize_elf_syms(linker);
+
 	err = finalize_btf(linker);
 	if (err)
 		return err;
@@ -2602,6 +2620,18 @@ int bpf_linker__finalize(struct bpf_linker *linker)
 	return 0;
 }
 
+static void finalize_elf_syms(struct bpf_linker *linker)
+{
+	struct dst_sec *symtab = &linker->secs[linker->symtab_sec_idx];
+	Elf64_Sym *sym = symtab->raw_data;
+	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
+
+	for (i = 0; i < n; i++, sym++) {
+		if (ELF64_ST_VISIBILITY(sym->st_other) == STV_HIDDEN)
+			sym_update_visibility(sym, STV_INTERNAL);
+	}
+}
+
 static int emit_elf_data_sec(struct bpf_linker *linker, const char *sec_name,
 			     size_t align, const void *raw_data, size_t raw_sz)
 {
-- 
2.30.2