[PATCH v5 bpf-next 0/5] libbpf: name-based u[ret]probe attach

[PATCH v5 bpf-next 0/5] libbpf: name-based u[ret]probe attach

[Alan Maguire]

This patch series focuses on supporting name-based attach - similar
to that supported for kprobes - for uprobe BPF programs.

Currently attach for such probes is done by determining the offset
manually, so the aim is to try and mimic the simplicity of kprobe
attach, making use of uprobe opts to specify a name string.
Patch 1 supports expansion of the binary_path argument used for
bpf_program__attach_uprobe_opts(), allowing it to determine paths
for programs and shared objects automatically, allowing for
specification of "libc.so.6" rather than the full path
"/usr/lib64/libc.so.6".

Patch 2 adds the "func_name" option to allow uprobe attach by
name; the mechanics are described there.

Having name-based support allows us to support auto-attach for
uprobes; patch 3 adds auto-attach support while attempting
to handle backwards-compatibility issues that arise.  The format
supported is

u[ret]probe/binary_path:[raw_offset|function[+offset]]

For example, to attach to libc malloc:

SEC("uprobe//usr/lib64/libc.so.6:malloc")

..or, making use of the path computation mechanisms introduced in patch 1

SEC("uprobe/libc.so.6:malloc")

Finally patch 4 add tests to the attach_probe selftests covering
attach by name, with patch 5 covering skeleton auto-attach.

Changes since v4 [1]:
- replaced strtok_r() usage with copying segments from static char *; avoids
  unneeded string allocation (Andrii, patch 1)
- switched to using access() instead of stat() when checking path-resolved
  binary (Andrii, patch 1)
- removed computation of .plt offset for instrumenting shared library calls
  within binaries.  Firstly it proved too brittle, and secondly it was somewhat
  unintuitive in that this form of instrumentation did not support function+offset
  as the "local function in binary" and "shared library function in shared library"
  cases did.  We can still instrument library calls, just need to do it in the
  library .so (patch 2)
- added binary path logging in cases where it was missing (Andrii, patch 2)
- avoid strlen() calcuation in checking name match (Andrii, patch 2)
- reword comments for func_name option (Andrii, patch 2)
- tightened SEC() name validation to support "u[ret]probe" and fail on other
  permutations that do not support auto-attach (i.e. have u[ret]probe/binary_path:func
  format (Andrii, patch 3)
- fixed selftests to fail independently rather than skip remainder on failure
  (Andrii, patches 4,5)
Changes since v3 [2]:
- reworked variable naming to fit better with libbpf conventions
  (Andrii, patch 2)
- use quoted binary path in log messages (Andrii, patch 2)
- added path determination mechanisms using LD_LIBRARY_PATH/PATH and
  standard locations (patch 1, Andrii)
- changed section lookup to be type+name (if name is specified) to
  simplify use cases (patch 2, Andrii)
- fixed .plt lookup scheme to match symbol table entries with .plt
  index via the .rela.plt table; also fix the incorrect assumption
  that the code in the .plt that does library linking is the same
  size as .plt entries (it just happens to be on x86_64)
- aligned with pluggable section support such that uprobe SEC() names
  that do not conform to auto-attach format do not cause skeleton load
  failure (patch 3, Andrii)
- no longer need to look up absolute path to libraries used by test_progs
  since we have mechanism to determine path automatically
- replaced CHECK()s with ASSERT*()s for attach_probe test (Andrii, patch 4)
- added auto-attach selftests also (Andrii, patch 5)
Changes since RFC [3]:
- used "long" for addresses instead of ssize_t (Andrii, patch 1).
- used gelf_ interfaces to avoid assumptions about 64-bit
  binaries (Andrii, patch 1)
- clarified string matching in symbol table lookups
  (Andrii, patch 1)
- added support for specification of shared object functions
  in a non-shared object binary.  This approach instruments
  the Procedure Linking Table (PLT) - malloc@PLT.
- changed logic in symbol search to check dynamic symbol table
  first, then fall back to symbol table (Andrii, patch 1).
- modified auto-attach string to require "/" separator prior
  to path prefix i.e. uprobe//path/to/binary (Andrii, patch 2)
- modified auto-attach string to use ':' separator (Andrii,
  patch 2)
- modified auto-attach to support raw offset (Andrii, patch 2)
- modified skeleton attach to interpret -ESRCH errors as
  a non-fatal "unable to auto-attach" (Andrii suggested
  -EOPNOTSUPP but my concern was it might collide with other
  instances where that value is returned and reflects a
  failure to attach a to-be-expected attachment rather than
  skip a program that does not present an auto-attachable
  section name. Admittedly -EOPNOTSUPP seems a more natural
  value here).
- moved library path retrieval code to trace_helpers (Andrii,
  patch 3)

[1] https://lore.kernel.org/bpf/1647000658-16149-1-git-send-email-alan.maguire@oracle.com/
[2] https://lore.kernel.org/bpf/1643645554-28723-1-git-send-email-alan.maguire@oracle.com/
[3] https://lore.kernel.org/bpf/1642678950-19584-1-git-send-email-alan.maguire@oracle.com/

Alan Maguire (5):
  libbpf: bpf_program__attach_uprobe_opts() should determine paths for
    programs/libraries where possible
  libbpf: support function name-based attach uprobes
  libbpf: add auto-attach for uprobes based on section name
  selftests/bpf: add tests for u[ret]probe attach by name
  selftests/bpf: add tests for uprobe auto-attach via skeleton

 tools/lib/bpf/libbpf.c                             | 330 ++++++++++++++++++++-
 tools/lib/bpf/libbpf.h                             |  10 +-
 .../selftests/bpf/prog_tests/attach_probe.c        |  85 +++++-
 .../selftests/bpf/prog_tests/uprobe_autoattach.c   |  38 +++
 .../selftests/bpf/progs/test_attach_probe.c        |  41 ++-
 .../selftests/bpf/progs/test_uprobe_autoattach.c   |  52 ++++
 6 files changed, 535 insertions(+), 21 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c

-- 
1.8.3.1

[PATCH v5 bpf-next 1/5] libbpf: bpf_program__attach_uprobe_opts() should determine paths for programs/libraries where possible

[Alan Maguire]

bpf_program__attach_uprobe_opts() requires a binary_path argument
specifying binary to instrument.  Supporting simply specifying
"libc.so.6" or "foo" should be possible too.

Library search checks LD_LIBRARY_PATH, then /usr/lib64, /usr/lib.
This allows users to run BPF programs prefixed with
LD_LIBRARY_PATH=/path2/lib while still searching standard locations.
Similarly for non .so files, we check PATH and /usr/bin, /usr/sbin.

Path determination will be useful for auto-attach of BPF uprobe programs
using SEC() definition.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/libbpf.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 809fe20..a7d5954 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10517,6 +10517,46 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
 	return pfd;
 }
 
+/* Get full path to program/shared library. */
+static int resolve_full_path(const char *file, char *result, size_t result_sz)
+{
+	const char *search_paths[2];
+	int i;
+
+	if (strstr(file, ".so")) {
+		search_paths[0] = getenv("LD_LIBRARY_PATH");
+		search_paths[1] = "/usr/lib64:/usr/lib";
+	} else {
+		search_paths[0] = getenv("PATH");
+		search_paths[1] = "/usr/bin:/usr/sbin";
+	}
+
+	for (i = 0; i < ARRAY_SIZE(search_paths); i++) {
+		const char *s;
+
+		if (!search_paths[i])
+			continue;
+		for (s = search_paths[i]; s != NULL; s = strchr(s, ':')) {
+			char *next_path;
+			int seg_len;
+
+			if (s[0] == ':')
+				s++;
+			next_path = strchr(s, ':');
+			seg_len = next_path ? next_path - s : strlen(s);
+			if (!seg_len)
+				continue;
+			snprintf(result, result_sz, "%.*s/%s", seg_len, s, file);
+			/* ensure it is an executable file/link */
+			if (access(result, R_OK | X_OK) < 0)
+				continue;
+			pr_debug("resolved '%s' to '%s'\n", file, result);
+			return 0;
+		}
+	}
+	return -ENOENT;
+}
+
 LIBBPF_API struct bpf_link *
 bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
 				const char *binary_path, size_t func_offset,
@@ -10524,6 +10564,7 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
 {
 	DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, pe_opts);
 	char errmsg[STRERR_BUFSIZE], *legacy_probe = NULL;
+	char full_binary_path[PATH_MAX];
 	struct bpf_link *link;
 	size_t ref_ctr_off;
 	int pfd, err;
@@ -10536,12 +10577,22 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
 	ref_ctr_off = OPTS_GET(opts, ref_ctr_offset, 0);
 	pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
 
+	if (binary_path && !strchr(binary_path, '/')) {
+		err = resolve_full_path(binary_path, full_binary_path,
+					sizeof(full_binary_path));
+		if (err) {
+			pr_warn("failed to resolve full path for '%s'\n", binary_path);
+			return libbpf_err_ptr(err);
+		}
+		binary_path = full_binary_path;
+	}
+
 	legacy = determine_uprobe_perf_type() < 0;
 	if (!legacy) {
 		pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path,
 					    func_offset, pid, ref_ctr_off);
 	} else {
-		char probe_name[512];
+		char probe_name[PATH_MAX + 64];
 
 		if (ref_ctr_off)
 			return libbpf_err_ptr(-EINVAL);
-- 
1.8.3.1

[PATCH v5 bpf-next 2/5] libbpf: support function name-based attach uprobes

[Alan Maguire]

kprobe attach is name-based, using lookups of kallsyms to translate
a function name to an address.  Currently uprobe attach is done
via an offset value as described in [1].  Extend uprobe opts
for attach to include a function name which can then be converted
into a uprobe-friendly offset.  The calcualation is done in
several steps:

1. First, determine the symbol address using libelf; this gives us
   the offset as reported by objdump
2. If the function is a shared library function - and the binary
   provided is a shared library - no further work is required;
   the address found is the required address
3. Finally, if the function is local, subtract the base address
   associated with the object, retrieved from ELF program headers.

The resultant value is then added to the func_offset value passed
in to specify the uprobe attach address.  So specifying a func_offset
of 0 along with a function name "printf" will attach to printf entry.

The modes of operation supported are then

1. to attach to a local function in a binary; function "foo1" in
   "/usr/bin/foo"
2. to attach to a shared library function in a shared library -
   function "malloc" in libc.

[1] https://www.kernel.org/doc/html/latest/trace/uprobetracer.html

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/libbpf.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h |  10 ++-
 2 files changed, 212 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a7d5954..eda724c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10517,6 +10517,195 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
 	return pfd;
 }
 
+/* uprobes deal in relative offsets; subtract the base address associated with
+ * the mapped binary.  See Documentation/trace/uprobetracer.rst for more
+ * details.
+ */
+static long elf_find_relative_offset(const char *filename, Elf *elf, long addr)
+{
+	size_t n;
+	int i;
+
+	if (elf_getphdrnum(elf, &n)) {
+		pr_warn("elf: failed to find program headers for '%s': %s\n", filename,
+			elf_errmsg(-1));
+		return -ENOENT;
+	}
+
+	for (i = 0; i < n; i++) {
+		int seg_start, seg_end, seg_offset;
+		GElf_Phdr phdr;
+
+		if (!gelf_getphdr(elf, i, &phdr)) {
+			pr_warn("elf: failed to get program header %d from '%s': %s\n", i, filename,
+				elf_errmsg(-1));
+			return -ENOENT;
+		}
+		if (phdr.p_type != PT_LOAD || !(phdr.p_flags & PF_X))
+			continue;
+
+		seg_start = phdr.p_vaddr;
+		seg_end = seg_start + phdr.p_memsz;
+		seg_offset = phdr.p_offset;
+		if (addr >= seg_start && addr < seg_end)
+			return addr - seg_start + seg_offset;
+	}
+	pr_warn("elf: failed to find prog header containing 0x%lx in '%s'\n", addr, filename);
+	return -ENOENT;
+}
+
+/* Return next ELF section of sh_type after scn, or first of that type if scn is NULL. */
+static Elf_Scn *elf_find_next_scn_by_type(Elf *elf, int sh_type, Elf_Scn *scn)
+{
+	while ((scn = elf_nextscn(elf, scn)) != NULL) {
+		GElf_Shdr sh;
+
+		if (!gelf_getshdr(scn, &sh))
+			continue;
+		if (sh.sh_type == sh_type)
+			return scn;
+	}
+	return NULL;
+}
+
+/* Find offset of function name in object specified by path.  "name" matches
+ * symbol name or name@@LIB for library functions.
+ */
+static long elf_find_func_offset(const char *binary_path, const char *name)
+{
+	int fd, i, sh_types[2] = { SHT_DYNSYM, SHT_SYMTAB };
+	bool is_shared_lib, is_name_qualified;
+	char errmsg[STRERR_BUFSIZE];
+	long ret = -ENOENT;
+	size_t name_len;
+	GElf_Ehdr ehdr;
+	Elf *elf;
+
+	fd = open(binary_path, O_RDONLY | O_CLOEXEC);
+	if (fd < 0) {
+		ret = -errno;
+		pr_warn("failed to open %s: %s\n", binary_path,
+			libbpf_strerror_r(ret, errmsg, sizeof(errmsg)));
+		return ret;
+	}
+	elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
+	if (!elf) {
+		pr_warn("elf: could not read elf from %s: %s\n", binary_path, elf_errmsg(-1));
+		close(fd);
+		return -LIBBPF_ERRNO__FORMAT;
+	}
+	if (!gelf_getehdr(elf, &ehdr)) {
+		pr_warn("elf: failed to get ehdr from %s: %s\n", binary_path, elf_errmsg(-1));
+		ret = -LIBBPF_ERRNO__FORMAT;
+		goto out;
+	}
+	/* for shared lib case, we do not need to calculate relative offset */
+	is_shared_lib = ehdr.e_type == ET_DYN;
+
+	name_len = strlen(name);
+	/* Does name specify "@@LIB"? */
+	is_name_qualified = strstr(name, "@@") != 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
+	 * linked binary may not have SHT_DYMSYM, so absence of a section should not be
+	 * reported as a warning/error.
+	 */
+	for (i = 0; i < ARRAY_SIZE(sh_types); i++) {
+		size_t nr_syms, strtabidx, idx;
+		Elf_Data *symbols = NULL;
+		Elf_Scn *scn = NULL;
+		int last_bind = -1;
+		const char *sname;
+		GElf_Shdr sh;
+
+		scn = elf_find_next_scn_by_type(elf, sh_types[i], NULL);
+		if (!scn) {
+			pr_debug("elf: failed to find symbol table ELF sections in '%s'\n",
+				 binary_path);
+			continue;
+		}
+		if (!gelf_getshdr(scn, &sh))
+			continue;
+		strtabidx = sh.sh_link;
+		symbols = elf_getdata(scn, 0);
+		if (!symbols) {
+			pr_warn("elf: failed to get symbols for symtab section in '%s': %s\n",
+				binary_path, elf_errmsg(-1));
+			ret = -LIBBPF_ERRNO__FORMAT;
+			goto out;
+		}
+		nr_syms = symbols->d_size / sh.sh_entsize;
+
+		for (idx = 0; idx < nr_syms; idx++) {
+			int curr_bind;
+			GElf_Sym sym;
+
+			if (!gelf_getsym(symbols, idx, &sym))
+				continue;
+
+			if (GELF_ST_TYPE(sym.st_info) != STT_FUNC)
+				continue;
+
+			sname = elf_strptr(elf, strtabidx, sym.st_name);
+			if (!sname)
+				continue;
+
+			curr_bind = GELF_ST_BIND(sym.st_info);
+
+			/* User can specify func, func@@LIB or func@@LIB_VERSION. */
+			if (strncmp(sname, 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 && sname[name_len] != '\0' && sname[name_len] != '@')
+				continue;
+
+			if (ret >= 0) {
+				/* handle multiple matches */
+				if (last_bind != STB_WEAK && curr_bind != STB_WEAK) {
+					/* Only accept one non-weak bind. */
+					pr_warn("elf: ambiguous match for '%s', '%s' in '%s'\n",
+						sname, name, binary_path);
+					ret = -LIBBPF_ERRNO__FORMAT;
+					goto out;
+				} else if (curr_bind == STB_WEAK) {
+					/* already have a non-weak bind, and
+					 * this is a weak bind, so ignore.
+					 */
+					continue;
+				}
+			}
+			ret = sym.st_value;
+			last_bind = curr_bind;
+		}
+		/* For binaries that are not shared libraries, we need relative offset */
+		if (ret > 0 && !is_shared_lib)
+			ret = elf_find_relative_offset(binary_path, elf, ret);
+		if (ret > 0)
+			break;
+	}
+
+	if (ret > 0) {
+		pr_debug("elf: symbol address match for '%s' in '%s': 0x%lx\n", name, binary_path,
+			 ret);
+	} else {
+		if (ret == 0) {
+			pr_warn("elf: '%s' is 0 in symtab for '%s': %s\n", name, binary_path,
+				is_shared_lib ? "should not be 0 in a shared library" :
+						"try using shared library path instead");
+			ret = -ENOENT;
+		} else {
+			pr_warn("elf: failed to find symbol '%s' in '%s'\n", name, binary_path);
+		}
+	}
+out:
+	elf_end(elf);
+	close(fd);
+	return ret;
+}
+
 /* Get full path to program/shared library. */
 static int resolve_full_path(const char *file, char *result, size_t result_sz)
 {
@@ -10569,6 +10758,7 @@ static int resolve_full_path(const char *file, char *result, size_t result_sz)
 	size_t ref_ctr_off;
 	int pfd, err;
 	bool retprobe, legacy;
+	const char *func_name;
 
 	if (!OPTS_VALID(opts, bpf_uprobe_opts))
 		return libbpf_err_ptr(-EINVAL);
@@ -10586,6 +10776,19 @@ static int resolve_full_path(const char *file, char *result, size_t result_sz)
 		}
 		binary_path = full_binary_path;
 	}
+	func_name = OPTS_GET(opts, func_name, NULL);
+	if (func_name) {
+		long sym_off;
+
+		if (!binary_path) {
+			pr_warn("name-based attach requires binary_path\n");
+			return libbpf_err_ptr(-EINVAL);
+		}
+		sym_off = elf_find_func_offset(binary_path, func_name);
+		if (sym_off < 0)
+			return libbpf_err_ptr(sym_off);
+		func_offset += sym_off;
+	}
 
 	legacy = determine_uprobe_perf_type() < 0;
 	if (!legacy) {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 05dde85..28cd206 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -459,9 +459,17 @@ struct bpf_uprobe_opts {
 	__u64 bpf_cookie;
 	/* uprobe is return probe, invoked at function return time */
 	bool retprobe;
+	/* Function name to attach to.  Could be an unqualified ("abc") or library-qualified
+	 * "abc@LIBXYZ" name.  To specify function entry, func_name should be set while
+	 * func_offset argument to bpf_prog__attach_uprobe_opts() should be 0.  To trace an
+	 * offset within a function, specify func_name and use func_offset argument to specify
+	 * offset within the function.  Shared library functions must specify the shared library
+	 * binary_path.
+	 */
+	const char *func_name;
 	size_t :0;
 };
-#define bpf_uprobe_opts__last_field retprobe
+#define bpf_uprobe_opts__last_field func_name
 
 /**
  * @brief **bpf_program__attach_uprobe()** attaches a BPF program
-- 
1.8.3.1

[PATCH v5 bpf-next 3/5] libbpf: add auto-attach for uprobes based on section name

[Alan Maguire]

Now that u[ret]probes can use name-based specification, it makes
sense to add support for auto-attach based on SEC() definition.
The format proposed is

        SEC("u[ret]probe/binary:[raw_offset|[function_name[+offset]]")

For example, to trace malloc() in libc:

        SEC("uprobe/libc.so.6:malloc")

...or to trace function foo2 in /usr/bin/foo:

        SEC("uprobe//usr/bin/foo:foo2")

Auto-attach is done for all tasks (pid -1).  prog can be an absolute
path or simply a program/library name; in the latter case, we use
PATH/LD_LIBRARY_PATH to resolve the full path, falling back to
standard locations (/usr/bin:/usr/sbin or /usr/lib64:/usr/lib) if
the file is not found via environment-variable specified locations.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/lib/bpf/libbpf.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index eda724c..38b1c91 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8630,6 +8630,7 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
 }
 
 static int attach_kprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
@@ -8642,9 +8643,9 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
 	SEC_DEF("sk_reuseport/migrate",	SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
 	SEC_DEF("sk_reuseport",		SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT, SEC_ATTACHABLE | SEC_SLOPPY_PFX),
 	SEC_DEF("kprobe/",		KPROBE,	0, SEC_NONE, attach_kprobe),
-	SEC_DEF("uprobe/",		KPROBE,	0, SEC_NONE),
+	SEC_DEF("uprobe+",		KPROBE,	0, SEC_NONE, attach_uprobe),
 	SEC_DEF("kretprobe/",		KPROBE, 0, SEC_NONE, attach_kprobe),
-	SEC_DEF("uretprobe/",		KPROBE, 0, SEC_NONE),
+	SEC_DEF("uretprobe+",		KPROBE, 0, SEC_NONE, attach_uprobe),
 	SEC_DEF("kprobe.multi/",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
 	SEC_DEF("kretprobe.multi/",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
 	SEC_DEF("tc",			SCHED_CLS, 0, SEC_NONE),
@@ -10843,6 +10844,75 @@ static int resolve_full_path(const char *file, char *result, size_t result_sz)
 
 }
 
+/* Format of u[ret]probe section definition supporting auto-attach:
+ * u[ret]probe/binary:function[+offset]
+ *
+ * binary can be an absolute/relative path or a filename; the latter is resolved to a
+ * full binary path via bpf_program__attach_uprobe_opts.
+ *
+ * Specifying uprobe+ ensures we carry out strict matching; either "uprobe" must be
+ * specified (and auto-attach is not possible) or the above format is specified for
+ * auto-attach.
+ */
+static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
+{
+	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
+	char *func, *probe_name, *func_end;
+	char *func_name, binary_path[512];
+	unsigned long long raw_offset;
+	size_t offset = 0;
+	int n;
+
+	*link = NULL;
+
+	opts.retprobe = str_has_pfx(prog->sec_name, "uretprobe/");
+	if (opts.retprobe)
+		probe_name = prog->sec_name + sizeof("uretprobe/") - 1;
+	else
+		probe_name = prog->sec_name + sizeof("uprobe/") - 1;
+
+	/* handle SEC("u[ret]probe") - format is valid, but auto-attach is impossible. */
+	if (strlen(probe_name) == 0) {
+		pr_debug("section '%s' is old-style u[ret]probe/function, cannot auto-attach\n",
+			 prog->sec_name);
+		return 0;
+	}
+	snprintf(binary_path, sizeof(binary_path), "%s", probe_name);
+	/* ':' should be prior to function+offset */
+	func_name = strrchr(binary_path, ':');
+	if (!func_name) {
+		pr_warn("section '%s' missing ':function[+offset]' specification\n",
+			prog->sec_name);
+		return -EINVAL;
+	}
+	func_name[0] = '\0';
+	func_name++;
+	n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset);
+	if (n < 1) {
+		pr_warn("uprobe name '%s' is invalid\n", func_name);
+		return -EINVAL;
+	}
+	if (opts.retprobe && offset != 0) {
+		free(func);
+		pr_warn("uretprobes do not support offset specification\n");
+		return -EINVAL;
+	}
+
+	/* Is func a raw address? */
+	errno = 0;
+	raw_offset = strtoull(func, &func_end, 0);
+	if (!errno && !*func_end) {
+		free(func);
+		func = NULL;
+		offset = (size_t)raw_offset;
+	}
+	opts.func_name = func;
+
+	*link = bpf_program__attach_uprobe_opts(prog, -1, binary_path, offset, &opts);
+	free(func);
+	return 0;
+}
+
 struct bpf_link *bpf_program__attach_uprobe(const struct bpf_program *prog,
 					    bool retprobe, pid_t pid,
 					    const char *binary_path,
-- 
1.8.3.1

[PATCH v5 bpf-next 4/5] selftests/bpf: add tests for u[ret]probe attach by name

[Alan Maguire]

add tests that verify attaching by name for

1. local functions in a program
2. library functions in a shared object

...succeed for uprobe and uretprobes using new "func_name"
option for bpf_program__attach_uprobe_opts().  Also verify
auto-attach works where uprobe, path to binary and function
name are specified, but fails with -EOPNOTSUPP with a SEC
name that does not specify binary path/function.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 .../selftests/bpf/prog_tests/attach_probe.c        | 85 ++++++++++++++++++----
 .../selftests/bpf/progs/test_attach_probe.c        | 41 ++++++++++-
 2 files changed, 109 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index d48f6e5..c0c6d41 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -11,15 +11,22 @@ static void trigger_func(void)
 	asm volatile ("");
 }
 
+/* attach point for byname uprobe */
+static void trigger_func2(void)
+{
+	asm volatile ("");
+}
+
 void test_attach_probe(void)
 {
 	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
-	int duration = 0;
 	struct bpf_link *kprobe_link, *kretprobe_link;
 	struct bpf_link *uprobe_link, *uretprobe_link;
 	struct test_attach_probe* skel;
 	ssize_t uprobe_offset, ref_ctr_offset;
+	struct bpf_link *uprobe_err_link;
 	bool legacy;
+	char *mem;
 
 	/* Check if new-style kprobe/uprobe API is supported.
 	 * Kernels that support new FD-based kprobe and uprobe BPF attachment
@@ -43,9 +50,9 @@ void test_attach_probe(void)
 		return;
 
 	skel = test_attach_probe__open_and_load();
-	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
 		return;
-	if (CHECK(!skel->bss, "check_bss", ".bss wasn't mmap()-ed\n"))
+	if (!ASSERT_OK_PTR(skel->bss, "check_bss"))
 		goto cleanup;
 
 	kprobe_link = bpf_program__attach_kprobe(skel->progs.handle_kprobe,
@@ -90,25 +97,73 @@ void test_attach_probe(void)
 		goto cleanup;
 	skel->links.handle_uretprobe = uretprobe_link;
 
-	/* trigger & validate kprobe && kretprobe */
-	usleep(1);
+	/* verify auto-attach fails for old-style uprobe definition */
+	uprobe_err_link = bpf_program__attach(skel->progs.handle_uprobe_byname);
+	if (!ASSERT_EQ(libbpf_get_error(uprobe_err_link), -EOPNOTSUPP,
+		       "auto-attach should fail for old-style name"))
+		goto cleanup;
+
+	uprobe_opts.func_name = "trigger_func2";
+	uprobe_opts.retprobe = false;
+	uprobe_opts.ref_ctr_offset = 0;
+	skel->links.handle_uprobe_byname =
+			bpf_program__attach_uprobe_opts(skel->progs.handle_uprobe_byname,
+							0 /* this pid */,
+							"/proc/self/exe",
+							0, &uprobe_opts);
+	if (!ASSERT_OK_PTR(skel->links.handle_uprobe_byname, "attach_uprobe_byname"))
+		goto cleanup;
+
+	/* verify auto-attach works */
+	skel->links.handle_uretprobe_byname =
+			bpf_program__attach(skel->progs.handle_uretprobe_byname);
+	if (!ASSERT_OK_PTR(skel->links.handle_uretprobe_byname, "attach_uretprobe_byname"))
+		goto cleanup;
 
-	if (CHECK(skel->bss->kprobe_res != 1, "check_kprobe_res",
-		  "wrong kprobe res: %d\n", skel->bss->kprobe_res))
+	/* test attach by name for a library function, using the library
+	 * as the binary argument. libc.so.6 will be resolved via dlopen()/dlinfo().
+	 */
+	uprobe_opts.func_name = "malloc";
+	uprobe_opts.retprobe = false;
+	skel->links.handle_uprobe_byname2 =
+			bpf_program__attach_uprobe_opts(skel->progs.handle_uprobe_byname2,
+							0 /* this pid */,
+							"libc.so.6",
+							0, &uprobe_opts);
+	if (!ASSERT_OK_PTR(skel->links.handle_uprobe_byname2, "attach_uprobe_byname2"))
 		goto cleanup;
-	if (CHECK(skel->bss->kretprobe_res != 2, "check_kretprobe_res",
-		  "wrong kretprobe res: %d\n", skel->bss->kretprobe_res))
+
+	uprobe_opts.func_name = "free";
+	uprobe_opts.retprobe = true;
+	skel->links.handle_uretprobe_byname2 =
+			bpf_program__attach_uprobe_opts(skel->progs.handle_uretprobe_byname2,
+							-1 /* any pid */,
+							"libc.so.6",
+							0, &uprobe_opts);
+	if (!ASSERT_OK_PTR(skel->links.handle_uretprobe_byname2, "attach_uretprobe_byname2"))
 		goto cleanup;
 
+	/* trigger & validate kprobe && kretprobe */
+	usleep(1);
+
+	/* trigger & validate shared library u[ret]probes attached by name */
+	mem = malloc(1);
+	free(mem);
+
 	/* trigger & validate uprobe & uretprobe */
 	trigger_func();
 
-	if (CHECK(skel->bss->uprobe_res != 3, "check_uprobe_res",
-		  "wrong uprobe res: %d\n", skel->bss->uprobe_res))
-		goto cleanup;
-	if (CHECK(skel->bss->uretprobe_res != 4, "check_uretprobe_res",
-		  "wrong uretprobe res: %d\n", skel->bss->uretprobe_res))
-		goto cleanup;
+	/* trigger & validate uprobe attached by name */
+	trigger_func2();
+
+	ASSERT_EQ(skel->bss->kprobe_res, 1, "check_kprobe_res");
+	ASSERT_EQ(skel->bss->kretprobe_res, 2, "check_kretprobe_res");
+	ASSERT_EQ(skel->bss->uprobe_res, 3, "check_uprobe_res");
+	ASSERT_EQ(skel->bss->uretprobe_res, 4, "check_uretprobe_res");
+	ASSERT_EQ(skel->bss->uprobe_byname_res, 5, "check_uprobe_byname_res");
+	ASSERT_EQ(skel->bss->uretprobe_byname_res, 6, "check_uretprobe_byname_res");
+	ASSERT_EQ(skel->bss->uprobe_byname2_res, 7, "check_uprobe_byname2_res");
+	ASSERT_EQ(skel->bss->uretprobe_byname2_res, 8, "check_uretprobe_byname2_res");
 
 cleanup:
 	test_attach_probe__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
index 8056a4c..af994d1 100644
--- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -10,6 +10,10 @@
 int kretprobe_res = 0;
 int uprobe_res = 0;
 int uretprobe_res = 0;
+int uprobe_byname_res = 0;
+int uretprobe_byname_res = 0;
+int uprobe_byname2_res = 0;
+int uretprobe_byname2_res = 0;
 
 SEC("kprobe/sys_nanosleep")
 int handle_kprobe(struct pt_regs *ctx)
@@ -25,18 +29,51 @@ int BPF_KRETPROBE(handle_kretprobe)
 	return 0;
 }
 
-SEC("uprobe/trigger_func")
+SEC("uprobe")
 int handle_uprobe(struct pt_regs *ctx)
 {
 	uprobe_res = 3;
 	return 0;
 }
 
-SEC("uretprobe/trigger_func")
+SEC("uretprobe")
 int handle_uretprobe(struct pt_regs *ctx)
 {
 	uretprobe_res = 4;
 	return 0;
 }
 
+SEC("uprobe")
+int handle_uprobe_byname(struct pt_regs *ctx)
+{
+	uprobe_byname_res = 5;
+	return 0;
+}
+
+/* use auto-attach format for section definition. */
+SEC("uretprobe//proc/self/exe:trigger_func2")
+int handle_uretprobe_byname(struct pt_regs *ctx)
+{
+	uretprobe_byname_res = 6;
+	return 0;
+}
+
+SEC("uprobe")
+int handle_uprobe_byname2(struct pt_regs *ctx)
+{
+	unsigned int size = PT_REGS_PARM1(ctx);
+
+	/* verify malloc size */
+	if (size == 1)
+		uprobe_byname2_res = 7;
+	return 0;
+}
+
+SEC("uretprobe")
+int handle_uretprobe_byname2(struct pt_regs *ctx)
+{
+	uretprobe_byname2_res = 8;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
1.8.3.1

[PATCH v5 bpf-next 5/5] selftests/bpf: add tests for uprobe auto-attach via skeleton

[Alan Maguire]

tests that verify auto-attach works for function entry/return for
local functions in program and library functions in a library.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 .../selftests/bpf/prog_tests/uprobe_autoattach.c   | 38 ++++++++++++++++
 .../selftests/bpf/progs/test_uprobe_autoattach.c   | 52 ++++++++++++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
new file mode 100644
index 0000000..03b15d6
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022, Oracle and/or its affiliates. */
+
+#include <test_progs.h>
+#include "test_uprobe_autoattach.skel.h"
+
+/* uprobe attach point */
+static void autoattach_trigger_func(void)
+{
+	asm volatile ("");
+}
+
+void test_uprobe_autoattach(void)
+{
+	struct test_uprobe_autoattach *skel;
+	char *mem;
+
+	skel = test_uprobe_autoattach__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	if (!ASSERT_OK(test_uprobe_autoattach__attach(skel), "skel_attach"))
+		goto cleanup;
+
+	/* trigger & validate uprobe & uretprobe */
+	autoattach_trigger_func();
+
+	/* trigger & validate shared library u[ret]probes attached by name */
+	mem = malloc(1);
+	free(mem);
+
+	ASSERT_EQ(skel->bss->uprobe_byname_res, 1, "check_uprobe_byname_res");
+	ASSERT_EQ(skel->bss->uretprobe_byname_res, 2, "check_uretprobe_byname_res");
+	ASSERT_EQ(skel->bss->uprobe_byname2_res, 3, "check_uprobe_byname2_res");
+	ASSERT_EQ(skel->bss->uretprobe_byname2_res, 4, "check_uretprobe_byname2_res");
+cleanup:
+	test_uprobe_autoattach__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
new file mode 100644
index 0000000..b442fb5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022, Oracle and/or its affiliates. */
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+int uprobe_byname_res = 0;
+int uretprobe_byname_res = 0;
+int uprobe_byname2_res = 0;
+int uretprobe_byname2_res = 0;
+
+/* This program cannot auto-attach, but that should not stop other
+ * programs from attaching.
+ */
+SEC("uprobe")
+int handle_uprobe_noautoattach(struct pt_regs *ctx)
+{
+	return 0;
+}
+
+SEC("uprobe//proc/self/exe:autoattach_trigger_func")
+int handle_uprobe_byname(struct pt_regs *ctx)
+{
+	uprobe_byname_res = 1;
+	return 0;
+}
+
+SEC("uretprobe//proc/self/exe:autoattach_trigger_func")
+int handle_uretprobe_byname(struct pt_regs *ctx)
+{
+	uretprobe_byname_res = 2;
+	return 0;
+}
+
+
+SEC("uprobe/libc.so.6:malloc")
+int handle_uprobe_byname2(struct pt_regs *ctx)
+{
+	uprobe_byname2_res = 3;
+	return 0;
+}
+
+SEC("uretprobe/libc.so.6:free")
+int handle_uretprobe_byname2(struct pt_regs *ctx)
+{
+	uretprobe_byname2_res = 4;
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
1.8.3.1

Re: [PATCH v5 bpf-next 0/5] libbpf: name-based u[ret]probe attach

[Andrii Nakryiko]

On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> This patch series focuses on supporting name-based attach - similar
> to that supported for kprobes - for uprobe BPF programs.
>
> Currently attach for such probes is done by determining the offset
> manually, so the aim is to try and mimic the simplicity of kprobe
> attach, making use of uprobe opts to specify a name string.
> Patch 1 supports expansion of the binary_path argument used for
> bpf_program__attach_uprobe_opts(), allowing it to determine paths
> for programs and shared objects automatically, allowing for
> specification of "libc.so.6" rather than the full path
> "/usr/lib64/libc.so.6".
>
> Patch 2 adds the "func_name" option to allow uprobe attach by
> name; the mechanics are described there.
>
> Having name-based support allows us to support auto-attach for
> uprobes; patch 3 adds auto-attach support while attempting
> to handle backwards-compatibility issues that arise.  The format
> supported is
>
> u[ret]probe/binary_path:[raw_offset|function[+offset]]
>
> For example, to attach to libc malloc:
>
> SEC("uprobe//usr/lib64/libc.so.6:malloc")
>
> ..or, making use of the path computation mechanisms introduced in patch 1
>
> SEC("uprobe/libc.so.6:malloc")
>
> Finally patch 4 add tests to the attach_probe selftests covering
> attach by name, with patch 5 covering skeleton auto-attach.
>
> Changes since v4 [1]:
> - replaced strtok_r() usage with copying segments from static char *; avoids
>   unneeded string allocation (Andrii, patch 1)
> - switched to using access() instead of stat() when checking path-resolved
>   binary (Andrii, patch 1)
> - removed computation of .plt offset for instrumenting shared library calls
>   within binaries.  Firstly it proved too brittle, and secondly it was somewhat
>   unintuitive in that this form of instrumentation did not support function+offset
>   as the "local function in binary" and "shared library function in shared library"
>   cases did.  We can still instrument library calls, just need to do it in the
>   library .so (patch 2)

ah, that's too bad, it seemed like a nice and clever idea. What was
brittle? Curious to learn for the future.

The fact that function+offset isn't supported int this "mode" seems
totally fine to me, we can provide a nice descriptive error in this
case anyways.

Anyways, all the added functionality makes sense and we can further
improve on it if necessary and possible. I've found a few small issues
with your patches and fixed some of them while applying, please do a
follow up for the rest. Thanks, great work and great addition to
libbpf!


> - added binary path logging in cases where it was missing (Andrii, patch 2)
> - avoid strlen() calcuation in checking name match (Andrii, patch 2)
> - reword comments for func_name option (Andrii, patch 2)
> - tightened SEC() name validation to support "u[ret]probe" and fail on other
>   permutations that do not support auto-attach (i.e. have u[ret]probe/binary_path:func
>   format (Andrii, patch 3)
> - fixed selftests to fail independently rather than skip remainder on failure
>   (Andrii, patches 4,5)
> Changes since v3 [2]:
> - reworked variable naming to fit better with libbpf conventions
>   (Andrii, patch 2)
> - use quoted binary path in log messages (Andrii, patch 2)
> - added path determination mechanisms using LD_LIBRARY_PATH/PATH and
>   standard locations (patch 1, Andrii)
> - changed section lookup to be type+name (if name is specified) to
>   simplify use cases (patch 2, Andrii)
> - fixed .plt lookup scheme to match symbol table entries with .plt
>   index via the .rela.plt table; also fix the incorrect assumption
>   that the code in the .plt that does library linking is the same
>   size as .plt entries (it just happens to be on x86_64)
> - aligned with pluggable section support such that uprobe SEC() names
>   that do not conform to auto-attach format do not cause skeleton load
>   failure (patch 3, Andrii)
> - no longer need to look up absolute path to libraries used by test_progs
>   since we have mechanism to determine path automatically
> - replaced CHECK()s with ASSERT*()s for attach_probe test (Andrii, patch 4)
> - added auto-attach selftests also (Andrii, patch 5)
> Changes since RFC [3]:
> - used "long" for addresses instead of ssize_t (Andrii, patch 1).
> - used gelf_ interfaces to avoid assumptions about 64-bit
>   binaries (Andrii, patch 1)
> - clarified string matching in symbol table lookups
>   (Andrii, patch 1)
> - added support for specification of shared object functions
>   in a non-shared object binary.  This approach instruments
>   the Procedure Linking Table (PLT) - malloc@PLT.
> - changed logic in symbol search to check dynamic symbol table
>   first, then fall back to symbol table (Andrii, patch 1).
> - modified auto-attach string to require "/" separator prior
>   to path prefix i.e. uprobe//path/to/binary (Andrii, patch 2)
> - modified auto-attach string to use ':' separator (Andrii,
>   patch 2)
> - modified auto-attach to support raw offset (Andrii, patch 2)
> - modified skeleton attach to interpret -ESRCH errors as
>   a non-fatal "unable to auto-attach" (Andrii suggested
>   -EOPNOTSUPP but my concern was it might collide with other
>   instances where that value is returned and reflects a
>   failure to attach a to-be-expected attachment rather than
>   skip a program that does not present an auto-attachable
>   section name. Admittedly -EOPNOTSUPP seems a more natural
>   value here).
> - moved library path retrieval code to trace_helpers (Andrii,
>   patch 3)
>
> [1] https://lore.kernel.org/bpf/1647000658-16149-1-git-send-email-alan.maguire@oracle.com/
> [2] https://lore.kernel.org/bpf/1643645554-28723-1-git-send-email-alan.maguire@oracle.com/
> [3] https://lore.kernel.org/bpf/1642678950-19584-1-git-send-email-alan.maguire@oracle.com/
>
> Alan Maguire (5):
>   libbpf: bpf_program__attach_uprobe_opts() should determine paths for
>     programs/libraries where possible
>   libbpf: support function name-based attach uprobes
>   libbpf: add auto-attach for uprobes based on section name
>   selftests/bpf: add tests for u[ret]probe attach by name
>   selftests/bpf: add tests for uprobe auto-attach via skeleton
>
>  tools/lib/bpf/libbpf.c                             | 330 ++++++++++++++++++++-
>  tools/lib/bpf/libbpf.h                             |  10 +-
>  .../selftests/bpf/prog_tests/attach_probe.c        |  85 +++++-
>  .../selftests/bpf/prog_tests/uprobe_autoattach.c   |  38 +++
>  .../selftests/bpf/progs/test_attach_probe.c        |  41 ++-
>  .../selftests/bpf/progs/test_uprobe_autoattach.c   |  52 ++++
>  6 files changed, 535 insertions(+), 21 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
>
> --
> 1.8.3.1
>

Re: [PATCH v5 bpf-next 1/5] libbpf: bpf_program__attach_uprobe_opts() should determine paths for programs/libraries where possible

[Andrii Nakryiko]

On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> bpf_program__attach_uprobe_opts() requires a binary_path argument
> specifying binary to instrument.  Supporting simply specifying
> "libc.so.6" or "foo" should be possible too.
>
> Library search checks LD_LIBRARY_PATH, then /usr/lib64, /usr/lib.
> This allows users to run BPF programs prefixed with
> LD_LIBRARY_PATH=/path2/lib while still searching standard locations.
> Similarly for non .so files, we check PATH and /usr/bin, /usr/sbin.
>
> Path determination will be useful for auto-attach of BPF uprobe programs
> using SEC() definition.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---

This patch's subject was very long, I took the liberty to shorten it as:

libbpf: auto-resolve programs/libraries when necessary for uprobes

Please yell if you strongly object.

>  tools/lib/bpf/libbpf.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 809fe20..a7d5954 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10517,6 +10517,46 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
>         return pfd;
>  }
>
> +/* Get full path to program/shared library. */
> +static int resolve_full_path(const char *file, char *result, size_t result_sz)
> +{
> +       const char *search_paths[2];
> +       int i;
> +
> +       if (strstr(file, ".so")) {

this check can be too brittle. It will think that binary called
"i.am.sorry" will be re assumed a shared library. While you can always
subvert this logic, maybe checking for ".so" suffix (not a substring)
and, if no such suffix found, looking for ".so." substring as a second
check? "i.am.so.sorry" still would fool it, but that looks like a very
artificial stretch.

let's improve this in a follow up

> +               search_paths[0] = getenv("LD_LIBRARY_PATH");
> +               search_paths[1] = "/usr/lib64:/usr/lib";

[...]

>  {
>         DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, pe_opts);
>         char errmsg[STRERR_BUFSIZE], *legacy_probe = NULL;
> +       char full_binary_path[PATH_MAX];
>         struct bpf_link *link;
>         size_t ref_ctr_off;
>         int pfd, err;
> @@ -10536,12 +10577,22 @@ static int perf_event_uprobe_open_legacy(const char *probe_name, bool retprobe,
>         ref_ctr_off = OPTS_GET(opts, ref_ctr_offset, 0);
>         pe_opts.bpf_cookie = OPTS_GET(opts, bpf_cookie, 0);
>
> +       if (binary_path && !strchr(binary_path, '/')) {
> +               err = resolve_full_path(binary_path, full_binary_path,
> +                                       sizeof(full_binary_path));
> +               if (err) {
> +                       pr_warn("failed to resolve full path for '%s'\n", binary_path);

we use "prog '%s': " prefix for error messages within
bpf_program__attach_xxx() wherever possible. Fixed up while applying.





> +                       return libbpf_err_ptr(err);
> +               }
> +               binary_path = full_binary_path;
> +       }
> +
>         legacy = determine_uprobe_perf_type() < 0;
>         if (!legacy) {
>                 pfd = perf_event_open_probe(true /* uprobe */, retprobe, binary_path,
>                                             func_offset, pid, ref_ctr_off);
>         } else {
> -               char probe_name[512];
> +               char probe_name[PATH_MAX + 64];
>
>                 if (ref_ctr_off)
>                         return libbpf_err_ptr(-EINVAL);
> --
> 1.8.3.1
>

Re: [PATCH v5 bpf-next 2/5] libbpf: support function name-based attach uprobes

[Andrii Nakryiko]

On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> kprobe attach is name-based, using lookups of kallsyms to translate
> a function name to an address.  Currently uprobe attach is done
> via an offset value as described in [1].  Extend uprobe opts
> for attach to include a function name which can then be converted
> into a uprobe-friendly offset.  The calcualation is done in
> several steps:
>
> 1. First, determine the symbol address using libelf; this gives us
>    the offset as reported by objdump
> 2. If the function is a shared library function - and the binary
>    provided is a shared library - no further work is required;
>    the address found is the required address
> 3. Finally, if the function is local, subtract the base address
>    associated with the object, retrieved from ELF program headers.
>
> The resultant value is then added to the func_offset value passed
> in to specify the uprobe attach address.  So specifying a func_offset
> of 0 along with a function name "printf" will attach to printf entry.
>
> The modes of operation supported are then
>
> 1. to attach to a local function in a binary; function "foo1" in
>    "/usr/bin/foo"
> 2. to attach to a shared library function in a shared library -
>    function "malloc" in libc.
>
> [1] https://www.kernel.org/doc/html/latest/trace/uprobetracer.html
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/libbpf.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h |  10 ++-
>  2 files changed, 212 insertions(+), 1 deletion(-)
>

[...]

> @@ -10569,6 +10758,7 @@ static int resolve_full_path(const char *file, char *result, size_t result_sz)
>         size_t ref_ctr_off;
>         int pfd, err;
>         bool retprobe, legacy;
> +       const char *func_name;
>
>         if (!OPTS_VALID(opts, bpf_uprobe_opts))
>                 return libbpf_err_ptr(-EINVAL);
> @@ -10586,6 +10776,19 @@ static int resolve_full_path(const char *file, char *result, size_t result_sz)
>                 }
>                 binary_path = full_binary_path;
>         }
> +       func_name = OPTS_GET(opts, func_name, NULL);
> +       if (func_name) {
> +               long sym_off;
> +
> +               if (!binary_path) {
> +                       pr_warn("name-based attach requires binary_path\n");

same about prog '%s': prefix



> +                       return libbpf_err_ptr(-EINVAL);
> +               }
> +               sym_off = elf_find_func_offset(binary_path, func_name);
> +               if (sym_off < 0)
> +                       return libbpf_err_ptr(sym_off);
> +               func_offset += sym_off;
> +       }
>
>         legacy = determine_uprobe_perf_type() < 0;
>         if (!legacy) {
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 05dde85..28cd206 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -459,9 +459,17 @@ struct bpf_uprobe_opts {
>         __u64 bpf_cookie;
>         /* uprobe is return probe, invoked at function return time */
>         bool retprobe;
> +       /* Function name to attach to.  Could be an unqualified ("abc") or library-qualified
> +        * "abc@LIBXYZ" name.  To specify function entry, func_name should be set while
> +        * func_offset argument to bpf_prog__attach_uprobe_opts() should be 0.  To trace an
> +        * offset within a function, specify func_name and use func_offset argument to specify
> +        * offset within the function.  Shared library functions must specify the shared library
> +        * binary_path.
> +        */
> +       const char *func_name;
>         size_t :0;
>  };
> -#define bpf_uprobe_opts__last_field retprobe
> +#define bpf_uprobe_opts__last_field func_name
>
>  /**
>   * @brief **bpf_program__attach_uprobe()** attaches a BPF program
> --
> 1.8.3.1
>

Re: [PATCH v5 bpf-next 3/5] libbpf: add auto-attach for uprobes based on section name

[Andrii Nakryiko]

On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Now that u[ret]probes can use name-based specification, it makes
> sense to add support for auto-attach based on SEC() definition.
> The format proposed is
>
>         SEC("u[ret]probe/binary:[raw_offset|[function_name[+offset]]")
>
> For example, to trace malloc() in libc:
>
>         SEC("uprobe/libc.so.6:malloc")
>
> ...or to trace function foo2 in /usr/bin/foo:
>
>         SEC("uprobe//usr/bin/foo:foo2")
>
> Auto-attach is done for all tasks (pid -1).  prog can be an absolute
> path or simply a program/library name; in the latter case, we use
> PATH/LD_LIBRARY_PATH to resolve the full path, falling back to
> standard locations (/usr/bin:/usr/sbin or /usr/lib64:/usr/lib) if
> the file is not found via environment-variable specified locations.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/libbpf.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 72 insertions(+), 2 deletions(-)
>

[...]

> +static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> +{
> +       DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
> +       char *func, *probe_name, *func_end;
> +       char *func_name, binary_path[512];
> +       unsigned long long raw_offset;
> +       size_t offset = 0;
> +       int n;
> +
> +       *link = NULL;
> +
> +       opts.retprobe = str_has_pfx(prog->sec_name, "uretprobe/");
> +       if (opts.retprobe)
> +               probe_name = prog->sec_name + sizeof("uretprobe/") - 1;
> +       else
> +               probe_name = prog->sec_name + sizeof("uprobe/") - 1;

I think this will mishandle SEC("uretprobe"), let's fix this in a
follow up (and see a note about uretprobe selftests)

> +
> +       /* handle SEC("u[ret]probe") - format is valid, but auto-attach is impossible. */
> +       if (strlen(probe_name) == 0) {
> +               pr_debug("section '%s' is old-style u[ret]probe/function, cannot auto-attach\n",
> +                        prog->sec_name);

this seems excessive to log this, it's expected situation. The message
itself is also misleading, SEC("uretprobe") isn't old-style, it's
valid and supported case. SEC("uretprobe/something") is an error now,
so that's a different thing (let's improve handling in the follow up).

> +               return 0;
> +       }
> +       snprintf(binary_path, sizeof(binary_path), "%s", probe_name);
> +       /* ':' should be prior to function+offset */
> +       func_name = strrchr(binary_path, ':');
> +       if (!func_name) {
> +               pr_warn("section '%s' missing ':function[+offset]' specification\n",
> +                       prog->sec_name);
> +               return -EINVAL;
> +       }
> +       func_name[0] = '\0';
> +       func_name++;
> +       n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset);
> +       if (n < 1) {
> +               pr_warn("uprobe name '%s' is invalid\n", func_name);
> +               return -EINVAL;
> +       }

I have this feeling that you could have simplified this a bunch with
just one sscanf. Something along the lines of
"%m[^/]/%m[^:]:%m[a-zA-Z0-9_.]+%li". If one argument matched (supposed
to be uprobe or uretprobe), then it is a no-auto-attach case, just
exit. If two matched -- invalid definition (old-style definition you
were reporting erroneously above in pr_debug). If 3 matched -- binary
+ func (or abs offset), if 4 matched - binary + func + offset. That
should cover everything, right?

Please try to do this in a follow up.

> +       if (opts.retprobe && offset != 0) {
> +               free(func);
> +               pr_warn("uretprobes do not support offset specification\n");
> +               return -EINVAL;
> +       }
> +
> +       /* Is func a raw address? */
> +       errno = 0;
> +       raw_offset = strtoull(func, &func_end, 0);
> +       if (!errno && !*func_end) {
> +               free(func);
> +               func = NULL;
> +               offset = (size_t)raw_offset;
> +       }
> +       opts.func_name = func;
> +
> +       *link = bpf_program__attach_uprobe_opts(prog, -1, binary_path, offset, &opts);
> +       free(func);
> +       return 0;

this should have been return libbpf_get_error(*link), fixed it


> +}
> +
>  struct bpf_link *bpf_program__attach_uprobe(const struct bpf_program *prog,
>                                             bool retprobe, pid_t pid,
>                                             const char *binary_path,
> --
> 1.8.3.1
>

Re: [PATCH v5 bpf-next 5/5] selftests/bpf: add tests for uprobe auto-attach via skeleton

[Andrii Nakryiko]

On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> tests that verify auto-attach works for function entry/return for
> local functions in program and library functions in a library.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  .../selftests/bpf/prog_tests/uprobe_autoattach.c   | 38 ++++++++++++++++
>  .../selftests/bpf/progs/test_uprobe_autoattach.c   | 52 ++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> new file mode 100644
> index 0000000..03b15d6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2022, Oracle and/or its affiliates. */
> +
> +#include <test_progs.h>
> +#include "test_uprobe_autoattach.skel.h"
> +
> +/* uprobe attach point */
> +static void autoattach_trigger_func(void)
> +{
> +       asm volatile ("");
> +}
> +
> +void test_uprobe_autoattach(void)
> +{
> +       struct test_uprobe_autoattach *skel;
> +       char *mem;
> +
> +       skel = test_uprobe_autoattach__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "skel_open"))
> +               return;
> +
> +       if (!ASSERT_OK(test_uprobe_autoattach__attach(skel), "skel_attach"))
> +               goto cleanup;
> +
> +       /* trigger & validate uprobe & uretprobe */
> +       autoattach_trigger_func();
> +
> +       /* trigger & validate shared library u[ret]probes attached by name */
> +       mem = malloc(1);
> +       free(mem);
> +
> +       ASSERT_EQ(skel->bss->uprobe_byname_res, 1, "check_uprobe_byname_res");
> +       ASSERT_EQ(skel->bss->uretprobe_byname_res, 2, "check_uretprobe_byname_res");
> +       ASSERT_EQ(skel->bss->uprobe_byname2_res, 3, "check_uprobe_byname2_res");
> +       ASSERT_EQ(skel->bss->uretprobe_byname2_res, 4, "check_uretprobe_byname2_res");
> +cleanup:
> +       test_uprobe_autoattach__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> new file mode 100644
> index 0000000..b442fb5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_uprobe_autoattach.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2022, Oracle and/or its affiliates. */
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +int uprobe_byname_res = 0;
> +int uretprobe_byname_res = 0;
> +int uprobe_byname2_res = 0;
> +int uretprobe_byname2_res = 0;
> +
> +/* This program cannot auto-attach, but that should not stop other
> + * programs from attaching.
> + */
> +SEC("uprobe")
> +int handle_uprobe_noautoattach(struct pt_regs *ctx)
> +{
> +       return 0;
> +}
> +
> +SEC("uprobe//proc/self/exe:autoattach_trigger_func")
> +int handle_uprobe_byname(struct pt_regs *ctx)
> +{
> +       uprobe_byname_res = 1;
> +       return 0;
> +}
> +
> +SEC("uretprobe//proc/self/exe:autoattach_trigger_func")
> +int handle_uretprobe_byname(struct pt_regs *ctx)
> +{
> +       uretprobe_byname_res = 2;
> +       return 0;
> +}
> +
> +
> +SEC("uprobe/libc.so.6:malloc")
> +int handle_uprobe_byname2(struct pt_regs *ctx)
> +{
> +       uprobe_byname2_res = 3;
> +       return 0;
> +}
> +
> +SEC("uretprobe/libc.so.6:free")
> +int handle_uretprobe_byname2(struct pt_regs *ctx)
> +{
> +       uretprobe_byname2_res = 4;
> +       return 0;
> +}

I just realized that in all our uprobe tests we don't really check
that it was really a return probe. Can you please follow up with
changes to selftests that make relevant trigger functions return some
well-known value and uretprobe programs capturing and recording those
with user-space parts of selftests checking this?


> +
> +char _license[] SEC("license") = "GPL";
> --
> 1.8.3.1
>

Re: [PATCH v5 bpf-next 0/5] libbpf: name-based u[ret]probe attach

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Wed, 30 Mar 2022 16:26:35 +0100 you wrote:
> This patch series focuses on supporting name-based attach - similar
> to that supported for kprobes - for uprobe BPF programs.
> 
> Currently attach for such probes is done by determining the offset
> manually, so the aim is to try and mimic the simplicity of kprobe
> attach, making use of uprobe opts to specify a name string.
> Patch 1 supports expansion of the binary_path argument used for
> bpf_program__attach_uprobe_opts(), allowing it to determine paths
> for programs and shared objects automatically, allowing for
> specification of "libc.so.6" rather than the full path
> "/usr/lib64/libc.so.6".
> 
> [...]

Here is the summary with links:
  - [v5,bpf-next,1/5] libbpf: bpf_program__attach_uprobe_opts() should determine paths for programs/libraries where possible
    https://git.kernel.org/bpf/bpf-next/c/1ce3a60e3c28
  - [v5,bpf-next,2/5] libbpf: support function name-based attach uprobes
    https://git.kernel.org/bpf/bpf-next/c/433966e3ae04
  - [v5,bpf-next,3/5] libbpf: add auto-attach for uprobes based on section name
    https://git.kernel.org/bpf/bpf-next/c/7825470420d1
  - [v5,bpf-next,4/5] selftests/bpf: add tests for u[ret]probe attach by name
    https://git.kernel.org/bpf/bpf-next/c/5bc2f0c51181
  - [v5,bpf-next,5/5] selftests/bpf: add tests for uprobe auto-attach via skeleton
    https://git.kernel.org/bpf/bpf-next/c/948ef31c4cd9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v5 bpf-next 3/5] libbpf: add auto-attach for uprobes based on section name

[Andrii Nakryiko]

On Sun, Apr 3, 2022 at 6:14 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > Now that u[ret]probes can use name-based specification, it makes
> > sense to add support for auto-attach based on SEC() definition.
> > The format proposed is
> >
> >         SEC("u[ret]probe/binary:[raw_offset|[function_name[+offset]]")
> >
> > For example, to trace malloc() in libc:
> >
> >         SEC("uprobe/libc.so.6:malloc")
> >
> > ...or to trace function foo2 in /usr/bin/foo:
> >
> >         SEC("uprobe//usr/bin/foo:foo2")
> >
> > Auto-attach is done for all tasks (pid -1).  prog can be an absolute
> > path or simply a program/library name; in the latter case, we use
> > PATH/LD_LIBRARY_PATH to resolve the full path, falling back to
> > standard locations (/usr/bin:/usr/sbin or /usr/lib64:/usr/lib) if
> > the file is not found via environment-variable specified locations.
> >
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> >  tools/lib/bpf/libbpf.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 72 insertions(+), 2 deletions(-)
> >
>
> [...]
>
> > +static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > +{
> > +       DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
> > +       char *func, *probe_name, *func_end;
> > +       char *func_name, binary_path[512];
> > +       unsigned long long raw_offset;
> > +       size_t offset = 0;
> > +       int n;
> > +
> > +       *link = NULL;
> > +
> > +       opts.retprobe = str_has_pfx(prog->sec_name, "uretprobe/");
> > +       if (opts.retprobe)
> > +               probe_name = prog->sec_name + sizeof("uretprobe/") - 1;
> > +       else
> > +               probe_name = prog->sec_name + sizeof("uprobe/") - 1;
>
> I think this will mishandle SEC("uretprobe"), let's fix this in a
> follow up (and see a note about uretprobe selftests)

So I actually fixed it up a little bit to avoid test failure on s390x
arch. But now it's a different problem, complaining about not being
able to resolve libc.so.6. CC'ing Ilya, but I was wondering if it's
better to use more generic "libc.so" instead of "libc.so.6"? Have you
tried that?

We should also probably refactor attach_probe.c selftest to be a
collection of subtest, so that we can blacklist only some subtests.
For now I have to blacklist it entirely on s390x.

>
> > +
> > +       /* handle SEC("u[ret]probe") - format is valid, but auto-attach is impossible. */
> > +       if (strlen(probe_name) == 0) {
> > +               pr_debug("section '%s' is old-style u[ret]probe/function, cannot auto-attach\n",
> > +                        prog->sec_name);
>
> this seems excessive to log this, it's expected situation. The message
> itself is also misleading, SEC("uretprobe") isn't old-style, it's
> valid and supported case. SEC("uretprobe/something") is an error now,
> so that's a different thing (let's improve handling in the follow up).
>
> > +               return 0;
> > +       }
> > +       snprintf(binary_path, sizeof(binary_path), "%s", probe_name);
> > +       /* ':' should be prior to function+offset */
> > +       func_name = strrchr(binary_path, ':');
> > +       if (!func_name) {
> > +               pr_warn("section '%s' missing ':function[+offset]' specification\n",
> > +                       prog->sec_name);
> > +               return -EINVAL;
> > +       }
> > +       func_name[0] = '\0';
> > +       func_name++;
> > +       n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset);
> > +       if (n < 1) {
> > +               pr_warn("uprobe name '%s' is invalid\n", func_name);
> > +               return -EINVAL;
> > +       }
>
> I have this feeling that you could have simplified this a bunch with
> just one sscanf. Something along the lines of
> "%m[^/]/%m[^:]:%m[a-zA-Z0-9_.]+%li". If one argument matched (supposed
> to be uprobe or uretprobe), then it is a no-auto-attach case, just
> exit. If two matched -- invalid definition (old-style definition you
> were reporting erroneously above in pr_debug). If 3 matched -- binary
> + func (or abs offset), if 4 matched - binary + func + offset. That
> should cover everything, right?
>
> Please try to do this in a follow up.
>
> > +       if (opts.retprobe && offset != 0) {
> > +               free(func);
> > +               pr_warn("uretprobes do not support offset specification\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       /* Is func a raw address? */
> > +       errno = 0;
> > +       raw_offset = strtoull(func, &func_end, 0);
> > +       if (!errno && !*func_end) {
> > +               free(func);
> > +               func = NULL;
> > +               offset = (size_t)raw_offset;
> > +       }
> > +       opts.func_name = func;
> > +
> > +       *link = bpf_program__attach_uprobe_opts(prog, -1, binary_path, offset, &opts);
> > +       free(func);
> > +       return 0;
>
> this should have been return libbpf_get_error(*link), fixed it
>
>
> > +}
> > +
> >  struct bpf_link *bpf_program__attach_uprobe(const struct bpf_program *prog,
> >                                             bool retprobe, pid_t pid,
> >                                             const char *binary_path,
> > --
> > 1.8.3.1
> >

Re: [PATCH v5 bpf-next 3/5] libbpf: add auto-attach for uprobes based on section name

[Andrii Nakryiko]

On Sun, Apr 3, 2022 at 9:46 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Sun, Apr 3, 2022 at 6:14 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > >
> > > Now that u[ret]probes can use name-based specification, it makes
> > > sense to add support for auto-attach based on SEC() definition.
> > > The format proposed is
> > >
> > >         SEC("u[ret]probe/binary:[raw_offset|[function_name[+offset]]")
> > >
> > > For example, to trace malloc() in libc:
> > >
> > >         SEC("uprobe/libc.so.6:malloc")
> > >
> > > ...or to trace function foo2 in /usr/bin/foo:
> > >
> > >         SEC("uprobe//usr/bin/foo:foo2")
> > >
> > > Auto-attach is done for all tasks (pid -1).  prog can be an absolute
> > > path or simply a program/library name; in the latter case, we use
> > > PATH/LD_LIBRARY_PATH to resolve the full path, falling back to
> > > standard locations (/usr/bin:/usr/sbin or /usr/lib64:/usr/lib) if
> > > the file is not found via environment-variable specified locations.
> > >
> > > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > > ---
> > >  tools/lib/bpf/libbpf.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 72 insertions(+), 2 deletions(-)
> > >
> >
> > [...]
> >
> > > +static int attach_uprobe(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > > +{
> > > +       DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
> > > +       char *func, *probe_name, *func_end;
> > > +       char *func_name, binary_path[512];
> > > +       unsigned long long raw_offset;
> > > +       size_t offset = 0;
> > > +       int n;
> > > +
> > > +       *link = NULL;
> > > +
> > > +       opts.retprobe = str_has_pfx(prog->sec_name, "uretprobe/");
> > > +       if (opts.retprobe)
> > > +               probe_name = prog->sec_name + sizeof("uretprobe/") - 1;
> > > +       else
> > > +               probe_name = prog->sec_name + sizeof("uprobe/") - 1;
> >
> > I think this will mishandle SEC("uretprobe"), let's fix this in a
> > follow up (and see a note about uretprobe selftests)
>
> So I actually fixed it up a little bit to avoid test failure on s390x
> arch. But now it's a different problem, complaining about not being
> able to resolve libc.so.6. CC'ing Ilya, but I was wondering if it's
> better to use more generic "libc.so" instead of "libc.so.6"? Have you
> tried that?

See [0] for one such failure log.

  [0] https://github.com/kernel-patches/bpf/runs/5810017263?check_suite_focus=true

>
> We should also probably refactor attach_probe.c selftest to be a
> collection of subtest, so that we can blacklist only some subtests.
> For now I have to blacklist it entirely on s390x.
>
> >
> > > +
> > > +       /* handle SEC("u[ret]probe") - format is valid, but auto-attach is impossible. */
> > > +       if (strlen(probe_name) == 0) {
> > > +               pr_debug("section '%s' is old-style u[ret]probe/function, cannot auto-attach\n",
> > > +                        prog->sec_name);
> >
> > this seems excessive to log this, it's expected situation. The message
> > itself is also misleading, SEC("uretprobe") isn't old-style, it's
> > valid and supported case. SEC("uretprobe/something") is an error now,
> > so that's a different thing (let's improve handling in the follow up).
> >
> > > +               return 0;
> > > +       }
> > > +       snprintf(binary_path, sizeof(binary_path), "%s", probe_name);
> > > +       /* ':' should be prior to function+offset */
> > > +       func_name = strrchr(binary_path, ':');
> > > +       if (!func_name) {
> > > +               pr_warn("section '%s' missing ':function[+offset]' specification\n",
> > > +                       prog->sec_name);
> > > +               return -EINVAL;
> > > +       }
> > > +       func_name[0] = '\0';
> > > +       func_name++;
> > > +       n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset);
> > > +       if (n < 1) {
> > > +               pr_warn("uprobe name '%s' is invalid\n", func_name);
> > > +               return -EINVAL;
> > > +       }
> >
> > I have this feeling that you could have simplified this a bunch with
> > just one sscanf. Something along the lines of
> > "%m[^/]/%m[^:]:%m[a-zA-Z0-9_.]+%li". If one argument matched (supposed
> > to be uprobe or uretprobe), then it is a no-auto-attach case, just
> > exit. If two matched -- invalid definition (old-style definition you
> > were reporting erroneously above in pr_debug). If 3 matched -- binary
> > + func (or abs offset), if 4 matched - binary + func + offset. That
> > should cover everything, right?
> >
> > Please try to do this in a follow up.
> >
> > > +       if (opts.retprobe && offset != 0) {
> > > +               free(func);
> > > +               pr_warn("uretprobes do not support offset specification\n");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       /* Is func a raw address? */
> > > +       errno = 0;
> > > +       raw_offset = strtoull(func, &func_end, 0);
> > > +       if (!errno && !*func_end) {
> > > +               free(func);
> > > +               func = NULL;
> > > +               offset = (size_t)raw_offset;
> > > +       }
> > > +       opts.func_name = func;
> > > +
> > > +       *link = bpf_program__attach_uprobe_opts(prog, -1, binary_path, offset, &opts);
> > > +       free(func);
> > > +       return 0;
> >
> > this should have been return libbpf_get_error(*link), fixed it
> >
> >
> > > +}
> > > +
> > >  struct bpf_link *bpf_program__attach_uprobe(const struct bpf_program *prog,
> > >                                             bool retprobe, pid_t pid,
> > >                                             const char *binary_path,
> > > --
> > > 1.8.3.1
> > >

Re: [PATCH v5 bpf-next 3/5] libbpf: add auto-attach for uprobes based on section name

[Ilya Leoshkevich]

On Sun, 2022-04-03 at 21:46 -0700, Andrii Nakryiko wrote:
> On Sun, Apr 3, 2022 at 6:14 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> > 
> > On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire
> > <alan.maguire@oracle.com> wrote:
> > > 
> > > Now that u[ret]probes can use name-based specification, it makes
> > > sense to add support for auto-attach based on SEC() definition.
> > > The format proposed is
> > > 
> > >        
> > > SEC("u[ret]probe/binary:[raw_offset|[function_name[+offset]]")
> > > 
> > > For example, to trace malloc() in libc:
> > > 
> > >         SEC("uprobe/libc.so.6:malloc")
> > > 
> > > ...or to trace function foo2 in /usr/bin/foo:
> > > 
> > >         SEC("uprobe//usr/bin/foo:foo2")
> > > 
> > > Auto-attach is done for all tasks (pid -1).  prog can be an
> > > absolute
> > > path or simply a program/library name; in the latter case, we use
> > > PATH/LD_LIBRARY_PATH to resolve the full path, falling back to
> > > standard locations (/usr/bin:/usr/sbin or /usr/lib64:/usr/lib) if
> > > the file is not found via environment-variable specified
> > > locations.
> > > 
> > > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > > ---
> > >  tools/lib/bpf/libbpf.c | 74
> > > ++++++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 72 insertions(+), 2 deletions(-)
> > > 
> > 
> > [...]
> > 
> > > +static int attach_uprobe(const struct bpf_program *prog, long
> > > cookie, struct bpf_link **link)
> > > +{
> > > +       DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
> > > +       char *func, *probe_name, *func_end;
> > > +       char *func_name, binary_path[512];
> > > +       unsigned long long raw_offset;
> > > +       size_t offset = 0;
> > > +       int n;
> > > +
> > > +       *link = NULL;
> > > +
> > > +       opts.retprobe = str_has_pfx(prog->sec_name,
> > > "uretprobe/");
> > > +       if (opts.retprobe)
> > > +               probe_name = prog->sec_name +
> > > sizeof("uretprobe/") - 1;
> > > +       else
> > > +               probe_name = prog->sec_name + sizeof("uprobe/") -
> > > 1;
> > 
> > I think this will mishandle SEC("uretprobe"), let's fix this in a
> > follow up (and see a note about uretprobe selftests)
> 
> So I actually fixed it up a little bit to avoid test failure on s390x
> arch. But now it's a different problem, complaining about not being
> able to resolve libc.so.6. CC'ing Ilya, but I was wondering if it's
> better to use more generic "libc.so" instead of "libc.so.6"? Have you
> tried that?

I believe it's a Debian-specific issue (our s390x CI image is Debian).
libc is still called libc.so.6, but it's located in
/lib/s390x-linux-gnu.
This must also be an issue on Intel and other architectures.
I'll send a patch.

Re: [PATCH v5 bpf-next 3/5] libbpf: add auto-attach for uprobes based on section name

[Alan Maguire]

[-- Attachment #1: Type: text/plain, Size: 3812 bytes --]

On Mon, 4 Apr 2022, Ilya Leoshkevich wrote:

> On Sun, 2022-04-03 at 21:46 -0700, Andrii Nakryiko wrote:
> > On Sun, Apr 3, 2022 at 6:14 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > > 
> > > On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire
> > > <alan.maguire@oracle.com> wrote:
> > > > 
> > > > Now that u[ret]probes can use name-based specification, it makes
> > > > sense to add support for auto-attach based on SEC() definition.
> > > > The format proposed is
> > > > 
> > > >        
> > > > SEC("u[ret]probe/binary:[raw_offset|[function_name[+offset]]")
> > > > 
> > > > For example, to trace malloc() in libc:
> > > > 
> > > >         SEC("uprobe/libc.so.6:malloc")
> > > > 
> > > > ...or to trace function foo2 in /usr/bin/foo:
> > > > 
> > > >         SEC("uprobe//usr/bin/foo:foo2")
> > > > 
> > > > Auto-attach is done for all tasks (pid -1).  prog can be an
> > > > absolute
> > > > path or simply a program/library name; in the latter case, we use
> > > > PATH/LD_LIBRARY_PATH to resolve the full path, falling back to
> > > > standard locations (/usr/bin:/usr/sbin or /usr/lib64:/usr/lib) if
> > > > the file is not found via environment-variable specified
> > > > locations.
> > > > 
> > > > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > > > ---
> > > >  tools/lib/bpf/libbpf.c | 74
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++--
> > > >  1 file changed, 72 insertions(+), 2 deletions(-)
> > > > 
> > > 
> > > [...]
> > > 
> > > > +static int attach_uprobe(const struct bpf_program *prog, long
> > > > cookie, struct bpf_link **link)
> > > > +{
> > > > +       DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, opts);
> > > > +       char *func, *probe_name, *func_end;
> > > > +       char *func_name, binary_path[512];
> > > > +       unsigned long long raw_offset;
> > > > +       size_t offset = 0;
> > > > +       int n;
> > > > +
> > > > +       *link = NULL;
> > > > +
> > > > +       opts.retprobe = str_has_pfx(prog->sec_name,
> > > > "uretprobe/");
> > > > +       if (opts.retprobe)
> > > > +               probe_name = prog->sec_name +
> > > > sizeof("uretprobe/") - 1;
> > > > +       else
> > > > +               probe_name = prog->sec_name + sizeof("uprobe/") -
> > > > 1;
> > > 
> > > I think this will mishandle SEC("uretprobe"), let's fix this in a
> > > follow up (and see a note about uretprobe selftests)
> > 
> > So I actually fixed it up a little bit to avoid test failure on s390x
> > arch. But now it's a different problem, complaining about not being

Thanks for doing all the fix-ups Andrii, and to Ilya for the Debian/s390 
and selftests fixups!

> > able to resolve libc.so.6. CC'ing Ilya, but I was wondering if it's
> > better to use more generic "libc.so" instead of "libc.so.6"? Have you
> > tried that?
> 

I looked at that, and unfortunately it's tricky because on some platforms
libc.so is a text GNU ld config file - here's what it looks like on my 
system:

$ cat /usr/lib64/libc.so
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /lib64/libc.so.6 /usr/lib64/libc_nonshared.a  AS_NEEDED ( 
/lib64/ld-linux-x86-64.so.2 ) )

I tried the dlopen()/dlinfo() trick with libc.so, thinking we might be 
able to tap into native linking mechanisms such that it would parse 
that file, but it doesn't work for dlopen()ing libc.so unfortunately; 
it needed the .6 suffix.
 
> I believe it's a Debian-specific issue (our s390x CI image is Debian).
> libc is still called libc.so.6, but it's located in
> /lib/s390x-linux-gnu.
> This must also be an issue on Intel and other architectures.
> I'll send a patch.
> 

Thanks!

Re: [PATCH v5 bpf-next 0/5] libbpf: name-based u[ret]probe attach

[Alan Maguire]

On Mon, 4 Apr 2022, Andrii Nakryiko wrote:

> On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > This patch series focuses on supporting name-based attach - similar
> > to that supported for kprobes - for uprobe BPF programs.
> >
> > Currently attach for such probes is done by determining the offset
> > manually, so the aim is to try and mimic the simplicity of kprobe
> > attach, making use of uprobe opts to specify a name string.
> > Patch 1 supports expansion of the binary_path argument used for
> > bpf_program__attach_uprobe_opts(), allowing it to determine paths
> > for programs and shared objects automatically, allowing for
> > specification of "libc.so.6" rather than the full path
> > "/usr/lib64/libc.so.6".
> >
> > Patch 2 adds the "func_name" option to allow uprobe attach by
> > name; the mechanics are described there.
> >
> > Having name-based support allows us to support auto-attach for
> > uprobes; patch 3 adds auto-attach support while attempting
> > to handle backwards-compatibility issues that arise.  The format
> > supported is
> >
> > u[ret]probe/binary_path:[raw_offset|function[+offset]]
> >
> > For example, to attach to libc malloc:
> >
> > SEC("uprobe//usr/lib64/libc.so.6:malloc")
> >
> > ..or, making use of the path computation mechanisms introduced in patch 1
> >
> > SEC("uprobe/libc.so.6:malloc")
> >
> > Finally patch 4 add tests to the attach_probe selftests covering
> > attach by name, with patch 5 covering skeleton auto-attach.
> >
> > Changes since v4 [1]:
> > - replaced strtok_r() usage with copying segments from static char *; avoids
> >   unneeded string allocation (Andrii, patch 1)
> > - switched to using access() instead of stat() when checking path-resolved
> >   binary (Andrii, patch 1)
> > - removed computation of .plt offset for instrumenting shared library calls
> >   within binaries.  Firstly it proved too brittle, and secondly it was somewhat
> >   unintuitive in that this form of instrumentation did not support function+offset
> >   as the "local function in binary" and "shared library function in shared library"
> >   cases did.  We can still instrument library calls, just need to do it in the
> >   library .so (patch 2)
> 
> ah, that's too bad, it seemed like a nice and clever idea. What was
> brittle? Curious to learn for the future.
> 

On Ubuntu, Daniel reported selftest failures which corresponded to the 
cases where we attached to a library function in a non-library - i.e. used 
the .plt computations.  I reproduced this failure myself, and it seemed
that although we were correctly computing the size of the .plt initial
code - 16 bytes - and each .plt entry - again 16 bytes - finding the 
.rela.plt entry and using its index as the basis for figuring out which
.plt entry to instrument wasn't working. 
 
Specifically, the .rela.plt entry for "free" was 146, but the actual .plt 
location of free was the 372 entry in the .plt table.  I'm not clear on 
why, but the the correspondence betweeen .rela.plt and .plt order 
isn't present on Ubuntu.  

> The fact that function+offset isn't supported int this "mode" 
seems
> totally fine to me, we can provide a nice descriptive error in this
> case anyways.
> 

I'll try and figure out exactly what's going on above; would be nice if we 
can still add this in the future.

> Anyways, all the added functionality makes sense and we can further
> improve on it if necessary and possible. I've found a few small issues
> with your patches and fixed some of them while applying, please do a
> follow up for the rest.

Yep, will do, thanks for the fix-ups! Ilya has fixed a few of the issues 
too, so I'll have some follow-ups for the rest shortly. I'll take a look
at adding aarch64 to libbpf CI too, that would be great.

> Thanks, great work and great addition to
> libbpf!
> 

Thanks again!

Re: [PATCH v5 bpf-next 0/5] libbpf: name-based u[ret]probe attach

[Andrii Nakryiko]

On Tue, Apr 5, 2022 at 3:28 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Mon, 4 Apr 2022, Andrii Nakryiko wrote:
>
> > On Wed, Mar 30, 2022 at 8:27 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> > >
> > > This patch series focuses on supporting name-based attach - similar
> > > to that supported for kprobes - for uprobe BPF programs.
> > >
> > > Currently attach for such probes is done by determining the offset
> > > manually, so the aim is to try and mimic the simplicity of kprobe
> > > attach, making use of uprobe opts to specify a name string.
> > > Patch 1 supports expansion of the binary_path argument used for
> > > bpf_program__attach_uprobe_opts(), allowing it to determine paths
> > > for programs and shared objects automatically, allowing for
> > > specification of "libc.so.6" rather than the full path
> > > "/usr/lib64/libc.so.6".
> > >
> > > Patch 2 adds the "func_name" option to allow uprobe attach by
> > > name; the mechanics are described there.
> > >
> > > Having name-based support allows us to support auto-attach for
> > > uprobes; patch 3 adds auto-attach support while attempting
> > > to handle backwards-compatibility issues that arise.  The format
> > > supported is
> > >
> > > u[ret]probe/binary_path:[raw_offset|function[+offset]]
> > >
> > > For example, to attach to libc malloc:
> > >
> > > SEC("uprobe//usr/lib64/libc.so.6:malloc")
> > >
> > > ..or, making use of the path computation mechanisms introduced in patch 1
> > >
> > > SEC("uprobe/libc.so.6:malloc")
> > >
> > > Finally patch 4 add tests to the attach_probe selftests covering
> > > attach by name, with patch 5 covering skeleton auto-attach.
> > >
> > > Changes since v4 [1]:
> > > - replaced strtok_r() usage with copying segments from static char *; avoids
> > >   unneeded string allocation (Andrii, patch 1)
> > > - switched to using access() instead of stat() when checking path-resolved
> > >   binary (Andrii, patch 1)
> > > - removed computation of .plt offset for instrumenting shared library calls
> > >   within binaries.  Firstly it proved too brittle, and secondly it was somewhat
> > >   unintuitive in that this form of instrumentation did not support function+offset
> > >   as the "local function in binary" and "shared library function in shared library"
> > >   cases did.  We can still instrument library calls, just need to do it in the
> > >   library .so (patch 2)
> >
> > ah, that's too bad, it seemed like a nice and clever idea. What was
> > brittle? Curious to learn for the future.
> >
>
> On Ubuntu, Daniel reported selftest failures which corresponded to the
> cases where we attached to a library function in a non-library - i.e. used
> the .plt computations.  I reproduced this failure myself, and it seemed
> that although we were correctly computing the size of the .plt initial
> code - 16 bytes - and each .plt entry - again 16 bytes - finding the
> .rela.plt entry and using its index as the basis for figuring out which
> .plt entry to instrument wasn't working.
>
> Specifically, the .rela.plt entry for "free" was 146, but the actual .plt
> location of free was the 372 entry in the .plt table.  I'm not clear on
> why, but the the correspondence betweeen .rela.plt and .plt order
> isn't present on Ubuntu.

Ok, curious. I'm not a big expert on PLT and stuff, but would be
curious to look at such an ELF file and see if we are missing
something that can be recovered from PLT relocations maybe? If you
happen to have a small ELF with repro case, please share.

>
> > The fact that function+offset isn't supported int this "mode"
> seems
> > totally fine to me, we can provide a nice descriptive error in this
> > case anyways.
> >
>
> I'll try and figure out exactly what's going on above; would be nice if we
> can still add this in the future.

Yep, definitely, provided we are sure it will keep working reliably :)

>
> > Anyways, all the added functionality makes sense and we can further
> > improve on it if necessary and possible. I've found a few small issues
> > with your patches and fixed some of them while applying, please do a
> > follow up for the rest.
>
> Yep, will do, thanks for the fix-ups! Ilya has fixed a few of the issues
> too, so I'll have some follow-ups for the rest shortly. I'll take a look
> at adding aarch64 to libbpf CI too, that would be great.

Awesome, aarch64 seems like a logical addition, thanks!

>
> > Thanks, great work and great addition to
> > libbpf!
> >
>
> Thanks again!