Commit 831916fb authored by Andrii Nakryiko's avatar Andrii Nakryiko
Browse files

Merge branch 'libbpf: Support symbol versioning for uprobe' 

Hengqi Chen says:

====================
Dynamic symbols in shared library may have the same name, for example:

    $ nm -D /lib/x86_64-linux-gnu/libc.so.6 | grep rwlock_wrlock
    000000000009b1a0 T __pthread_rwlock_wrlock@GLIBC_2.2.5
    000000000009b1a0 T pthread_rwlock_wrlock@@GLIBC_2.34
    000000000009b1a0 T pthread_rwlock_wrlock@GLIBC_2.2.5

    $ readelf -W --dyn-syms /lib/x86_64-linux-gnu/libc.so.6 | grep rwlock_wrlock
      706: 000000000009b1a0   878 FUNC    GLOBAL DEFAULT   15 __pthread_rwlock_wrlock@GLIBC_2.2.5
      2568: 000000000009b1a0   878 FUNC    GLOBAL DEFAULT   15 pthread_rwlock_wrlock@@GLIBC_2.34
      2571: 000000000009b1a0   878 FUNC    GLOBAL DEFAULT   15 pthread_rwlock_wrlock@GLIBC_2.2.5

There are two pthread_rwlock_wrlock symbols in libc.so .dynsym section.
The one with @@ is the default version, the other is hidden.
Note that the version info is stored in .gnu.version and .gnu.version_d
sections of libc and the two symbols are at the _same_ offset.

Currently, specify `pthread_rwlock_wrlock`, `pthread_rwlock_wrlock@@GLIBC_2.34`
or `pthread_rwlock_wrlock@GLIBC_2.2.5` in bpf_uprobe_opts::func_name won't work.
Because there are two `pthread_rwlock_wrlock` in .dynsym sections without the
version suffix and both are global bind.

We could solve this by introducing symbol versioning ([0]). So that users can
specify func, func@LIB_VERSION or func@@LIB_VERSION to attach a uprobe.

This patchset resolves symbol conflicts and add symbol versioning for uprobe.
  - Patch 1 resolves symbol conflicts at the same offset
  - Patch 2 adds symbol versioning for dynsym
  - Patch 3 adds selftests for the above changes

Changes from v3:
  - Address comments from Andrii

Changes from v2:
  - Add uretprobe selfttest (Alan)
  - Check symbol exact match (Alan)
  - Fix typo (Jiri)

Changes from v1:
  - Address comments from Alan and Jiri
  - Add selftests (Someone reminds me that there is an attempt at [1]
    and part of the selftest code from Andrii is taken from there)

  [0]: https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/symversion.html
  [1]: https://lore.kernel.org/lkml/CAEf4BzZTrjjyyOm3ak9JsssPSh6T_ZmGd677a2rt5e5rBLUrpQ@mail.gmail.com/


====================

Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
parents ac0691c7 7089f85a

tools/lib/bpf/elf.c

+127 −12
Original line number Diff line number Diff line 
// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)



#ifndef _GNU_SOURCE

#define _GNU_SOURCE

#endif

#include <libelf.h>

#include <gelf.h>

#include <fcntl.h>
@@ -10,6 +13,17 @@ 


#define STRERR_BUFSIZE  128



/* A SHT_GNU_versym section holds 16-bit words. This bit is set if

 * the symbol is hidden and can only be seen when referenced using an

 * explicit version number. This is a GNU extension.

 */

#define VERSYM_HIDDEN	0x8000



/* This is the mask for the rest of the data in a word read from a

 * SHT_GNU_versym section.

 */

#define VERSYM_VERSION	0x7fff



int elf_open(const char *binary_path, struct elf_fd *elf_fd)

{

	char errmsg[STRERR_BUFSIZE];
@@ -64,13 +78,18 @@ struct elf_sym { 
	const char *name;

	GElf_Sym sym;

	GElf_Shdr sh;

	int ver;

	bool hidden;

};



struct elf_sym_iter {

	Elf *elf;

	Elf_Data *syms;

	Elf_Data *versyms;

	Elf_Data *verdefs;

	size_t nr_syms;

	size_t strtabidx;

	size_t verdef_strtabidx;

	size_t next_sym_idx;

	struct elf_sym sym;

	int st_type;
@@ -111,6 +130,26 @@ static int elf_sym_iter_new(struct elf_sym_iter *iter, 
	iter->nr_syms = iter->syms->d_size / sh.sh_entsize;

	iter->elf = elf;

	iter->st_type = st_type;



	/* Version symbol table is meaningful to dynsym only */

	if (sh_type != SHT_DYNSYM)

		return 0;



	scn = elf_find_next_scn_by_type(elf, SHT_GNU_versym, NULL);

	if (!scn)

		return 0;

	iter->versyms = elf_getdata(scn, 0);



	scn = elf_find_next_scn_by_type(elf, SHT_GNU_verdef, NULL);

	if (!scn) {

		pr_debug("elf: failed to find verdef ELF sections in '%s'\n", binary_path);

		return -ENOENT;

	}

	if (!gelf_getshdr(scn, &sh))

		return -EINVAL;

	iter->verdef_strtabidx = sh.sh_link;

	iter->verdefs = elf_getdata(scn, 0);



	return 0;

}
@@ -119,6 +158,7 @@ static struct elf_sym *elf_sym_iter_next(struct elf_sym_iter *iter) 
	struct elf_sym *ret = &iter->sym;

	GElf_Sym *sym = &ret->sym;

	const char *name = NULL;

	GElf_Versym versym;

	Elf_Scn *sym_scn;

	size_t idx;
@@ -138,12 +178,80 @@ static struct elf_sym *elf_sym_iter_next(struct elf_sym_iter *iter) 


		iter->next_sym_idx = idx + 1;

		ret->name = name;

		ret->ver = 0;

		ret->hidden = false;



		if (iter->versyms) {

			if (!gelf_getversym(iter->versyms, idx, &versym))

				continue;

			ret->ver = versym & VERSYM_VERSION;

			ret->hidden = versym & VERSYM_HIDDEN;

		}

		return ret;

	}



	return NULL;

}



static const char *elf_get_vername(struct elf_sym_iter *iter, int ver)

{

	GElf_Verdaux verdaux;

	GElf_Verdef verdef;

	int offset;



	offset = 0;

	while (gelf_getverdef(iter->verdefs, offset, &verdef)) {

		if (verdef.vd_ndx != ver) {

			if (!verdef.vd_next)

				break;



			offset += verdef.vd_next;

			continue;

		}



		if (!gelf_getverdaux(iter->verdefs, offset + verdef.vd_aux, &verdaux))

			break;



		return elf_strptr(iter->elf, iter->verdef_strtabidx, verdaux.vda_name);



	}

	return NULL;

}



static bool symbol_match(struct elf_sym_iter *iter, int sh_type, struct elf_sym *sym,

			 const char *name, size_t name_len, const char *lib_ver)

{

	const char *ver_name;



	/* Symbols are in forms of func, func@LIB_VER or func@@LIB_VER

	 * make sure the func part matches the user specified name

	 */

	if (strncmp(sym->name, name, name_len) != 0)

		return false;



	/* ...but we don't want a search for "foo" to match 'foo2" also, so any

	 * additional characters in sname should be of the form "@@LIB".

	 */

	if (sym->name[name_len] != '\0' && sym->name[name_len] != '@')

		return false;



	/* If user does not specify symbol version, then we got a match */

	if (!lib_ver)

		return true;



	/* If user specifies symbol version, for dynamic symbols,

	 * get version name from ELF verdef section for comparison.

	 */

	if (sh_type == SHT_DYNSYM) {

		ver_name = elf_get_vername(iter, sym->ver);

		if (!ver_name)

			return false;

		return strcmp(ver_name, lib_ver) == 0;

	}



	/* For normal symbols, it is already in form of func@LIB_VER */

	return strcmp(sym->name, name) == 0;

}



/* Transform symbol's virtual address (absolute for binaries and relative

 * for shared libs) into file offset, which is what kernel is expecting
@@ -166,7 +274,8 @@ static unsigned long elf_sym_offset(struct elf_sym *sym) 
long elf_find_func_offset(Elf *elf, const char *binary_path, const char *name)

{

	int i, sh_types[2] = { SHT_DYNSYM, SHT_SYMTAB };

	bool is_shared_lib, is_name_qualified;

	const char *at_symbol, *lib_ver;

	bool is_shared_lib;

	long ret = -ENOENT;

	size_t name_len;

	GElf_Ehdr ehdr;
@@ -179,9 +288,18 @@ long elf_find_func_offset(Elf *elf, const char *binary_path, const char *name) 
	/* for shared lib case, we do not need to calculate relative offset */

	is_shared_lib = ehdr.e_type == ET_DYN;



	/* Does name specify "@@LIB_VER" or "@LIB_VER" ? */

	at_symbol = strchr(name, '@');

	if (at_symbol) {

		name_len = at_symbol - name;

		/* skip second @ if it's @@LIB_VER case */

		if (at_symbol[1] == '@')

			at_symbol++;

		lib_ver = at_symbol + 1;

	} else {

		name_len = strlen(name);

	/* Does name specify "@@LIB"? */

	is_name_qualified = strstr(name, "@@") != NULL;

		lib_ver = NULL;

	}



	/* Search SHT_DYNSYM, SHT_SYMTAB for symbol. This search order is used because if

	 * a binary is stripped, it may only have SHT_DYNSYM, and a fully-statically
@@ -201,20 +319,17 @@ long elf_find_func_offset(Elf *elf, const char *binary_path, const char *name) 
			goto out;



		while ((sym = elf_sym_iter_next(&iter))) {

			/* User can specify func, func@@LIB or func@@LIB_VERSION. */

			if (strncmp(sym->name, name, name_len) != 0)

				continue;

			/* ...but we don't want a search for "foo" to match 'foo2" also, so any

			 * additional characters in sname should be of the form "@@LIB".

			 */

			if (!is_name_qualified && sym->name[name_len] != '\0' && sym->name[name_len] != '@')

			if (!symbol_match(&iter, sh_types[i], sym, name, name_len, lib_ver))

				continue;



			cur_bind = GELF_ST_BIND(sym->sym.st_info);



			if (ret > 0) {

				/* handle multiple matches */

				if (last_bind != STB_WEAK && cur_bind != STB_WEAK) {

				if (elf_sym_offset(sym) == ret) {

					/* same offset, no problem */

					continue;

				} else if (last_bind != STB_WEAK && cur_bind != STB_WEAK) {

					/* Only accept one non-weak bind. */

					pr_warn("elf: ambiguous match for '%s', '%s' in '%s'\n",

						sym->name, name, binary_path);

tools/lib/bpf/libbpf.c

+1 −1
Original line number Diff line number Diff line
@@ -11630,7 +11630,7 @@ static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf 


	*link = NULL;



	n = sscanf(prog->sec_name, "%m[^/]/%m[^:]:%m[a-zA-Z0-9_.]+%li",

	n = sscanf(prog->sec_name, "%m[^/]/%m[^:]:%m[a-zA-Z0-9_.@]+%li",

		   &probe_type, &binary_path, &func_name, &offset);

	switch (n) {

	case 1:

tools/testing/selftests/bpf/Makefile

+3 −2
Original line number Diff line number Diff line
@@ -196,11 +196,12 @@ endif 


# Filter out -static for liburandom_read.so and its dependent targets so that static builds

# do not fail. Static builds leave urandom_read relying on system-wide shared libraries.

$(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c

$(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c liburandom_read.map

	$(call msg,LIB,,$@)

	$(Q)$(CLANG) $(filter-out -static,$(CFLAGS) $(LDFLAGS))   \

		     $^ $(filter-out -static,$(LDLIBS))	     \

		     $(filter %.c,$^) $(filter-out -static,$(LDLIBS)) \

		     -fuse-ld=$(LLD) -Wl,-znoseparate-code -Wl,--build-id=sha1 \

		     -Wl,--version-script=liburandom_read.map \

		     -fPIC -shared -o $@



$(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so

tools/testing/selftests/bpf/liburandom_read.map

0 → 100644
+15 −0
Original line number Diff line number Diff line 
LIBURANDOM_READ_1.0.0 {

	global:

		urandlib_api;

		urandlib_api_sameoffset;

		urandlib_read_without_sema;

		urandlib_read_with_sema;

		urandlib_read_with_sema_semaphore;

	local:

		*;

};



LIBURANDOM_READ_2.0.0 {

	global:

		urandlib_api;

} LIBURANDOM_READ_1.0.0;

tools/testing/selftests/bpf/prog_tests/uprobe.c

0 → 100644
+95 −0
Original line number Diff line number Diff line 
// SPDX-License-Identifier: GPL-2.0

/* Copyright (c) 2023 Hengqi Chen */



#include <test_progs.h>

#include "test_uprobe.skel.h"



static FILE *urand_spawn(int *pid)

{

	FILE *f;



	/* urandom_read's stdout is wired into f */

	f = popen("./urandom_read 1 report-pid", "r");

	if (!f)

		return NULL;



	if (fscanf(f, "%d", pid) != 1) {

		pclose(f);

		errno = EINVAL;

		return NULL;

	}



	return f;

}



static int urand_trigger(FILE **urand_pipe)

{

	int exit_code;



	/* pclose() waits for child process to exit and returns their exit code */

	exit_code = pclose(*urand_pipe);

	*urand_pipe = NULL;



	return exit_code;

}



void test_uprobe(void)

{

	LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);

	struct test_uprobe *skel;

	FILE *urand_pipe = NULL;

	int urand_pid = 0, err;



	skel = test_uprobe__open_and_load();

	if (!ASSERT_OK_PTR(skel, "skel_open"))

		return;



	urand_pipe = urand_spawn(&urand_pid);

	if (!ASSERT_OK_PTR(urand_pipe, "urand_spawn"))

		goto cleanup;



	skel->bss->my_pid = urand_pid;



	/* Manual attach uprobe to urandlib_api

	 * There are two `urandlib_api` symbols in .dynsym section:

	 *   - urandlib_api@LIBURANDOM_READ_1.0.0

	 *   - urandlib_api@@LIBURANDOM_READ_2.0.0

	 * Both are global bind and would cause a conflict if user

	 * specify the symbol name without a version suffix

	 */

	uprobe_opts.func_name = "urandlib_api";

	skel->links.test4 = bpf_program__attach_uprobe_opts(skel->progs.test4,

							    urand_pid,

							    "./liburandom_read.so",

							    0 /* offset */,

							    &uprobe_opts);

	if (!ASSERT_ERR_PTR(skel->links.test4, "urandlib_api_attach_conflict"))

		goto cleanup;



	uprobe_opts.func_name = "urandlib_api@LIBURANDOM_READ_1.0.0";

	skel->links.test4 = bpf_program__attach_uprobe_opts(skel->progs.test4,

							    urand_pid,

							    "./liburandom_read.so",

							    0 /* offset */,

							    &uprobe_opts);

	if (!ASSERT_OK_PTR(skel->links.test4, "urandlib_api_attach_ok"))

		goto cleanup;



	/* Auto attach 3 u[ret]probes to urandlib_api_sameoffset */

	err = test_uprobe__attach(skel);

	if (!ASSERT_OK(err, "skel_attach"))

		goto cleanup;



	/* trigger urandom_read */

	ASSERT_OK(urand_trigger(&urand_pipe), "urand_exit_code");



	ASSERT_EQ(skel->bss->test1_result, 1, "urandlib_api_sameoffset");

	ASSERT_EQ(skel->bss->test2_result, 1, "urandlib_api_sameoffset@v1");

	ASSERT_EQ(skel->bss->test3_result, 3, "urandlib_api_sameoffset@@v2");

	ASSERT_EQ(skel->bss->test4_result, 1, "urandlib_api");



cleanup:

	if (urand_pipe)

		pclose(urand_pipe);

	test_uprobe__destroy(skel);

}