On Mon, Jan 31, 2022 at 8:13 AM Alan Maguire <alan.maguire@xxxxxxxxxx> 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; then, in the case of local > functions > 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. If the function is a shared library function in a program > (as opposed to a shared library), the Procedure Linking Table > (PLT) table address is found (it is indexed via the dynamic > symbol table index). This allows us to instrument a call > to a shared library function locally in the calling binary, > reducing overhead versus having a breakpoint in global lib. > 4. 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 binary; > function "malloc" in "/usr/bin/foo" > 3. 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@xxxxxxxxxx> > --- This looks great and very clean. I left a few nits, but otherwise it looks ready (still need to go through the rest of the patches) > tools/lib/bpf/libbpf.c | 250 +++++++++++++++++++++++++++++++++++++++++++++++++ > tools/lib/bpf/libbpf.h | 10 +- > 2 files changed, 259 insertions(+), 1 deletion(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 4ce94f4..eb95629 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -10203,6 +10203,241 @@ 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(Elf *elf, long addr) nit: too many spaces after comma > +{ > + size_t n; > + int i; > + > + if (elf_getphdrnum(elf, &n)) { > + pr_warn("elf: failed to find program headers: %s\n", > + 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: %s\n", > + i, 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; nit: double space before seg_start > + } > + pr_warn("elf: failed to find prog header containing 0x%lx\n", addr); > + 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) > + break; > + } > + return scn; > +} > + > +/* For Position-Independent Code-based libraries, a table of trampolines > + * (Procedure Linking Table) is used to support resolution of symbol > + * names at linking time. The goal here is to find the offset associated > + * with the jump to the actual library function. If we can instrument that > + * locally in the specific binary (rather than instrumenting glibc say), > + * overheads are greatly reduced. > + * > + * The method used is to find the .plt section and determine the offset > + * of the relevant entry (given by the base address plus the index > + * of the function multiplied by the size of a .plt entry). > + */ > +static ssize_t elf_find_plt_offset(Elf *elf, size_t ndx) nit: ndx -> func_idx? libbpf generally uses "idx" naming, "ndx" is purely libelf's convention (and it is more obvious if it is explicitly called out that it's index of a function entry) > +{ > + Elf_Scn *scn = NULL; > + size_t shstrndx; > + > + if (elf_getshdrstrndx(elf, &shstrndx)) { > + pr_debug("elf: failed to get section names section index: %s\n", > + elf_errmsg(-1)); > + return -LIBBPF_ERRNO__FORMAT; > + } > + while ((scn = elf_find_next_scn_by_type(elf, SHT_PROGBITS, scn))) { > + long plt_entry_sz, plt_base; > + const char *name; > + GElf_Shdr sh; > + > + if (!gelf_getshdr(scn, &sh)) > + continue; > + name = elf_strptr(elf, shstrndx, sh.sh_name); > + if (!name || strcmp(name, ".plt") != 0) > + continue; Wouldn't it be simpler to use elf_sec_by_name(elf, ".plt") and then Shdr and check PROGBITS? Given there will be only one .plt, it makes more sense than this while loop? > + plt_base = sh.sh_addr; > + plt_entry_sz = sh.sh_entsize; > + return plt_base + (ndx * plt_entry_sz); > + } > + pr_debug("elf: no .plt section found\n"); Do we really need this, especially without a binary path? > + return -LIBBPF_ERRNO__FORMAT; > +} > + > +/* 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; > + size_t name_len, sym_ndx = -1; > + char errmsg[STRERR_BUFSIZE]; > + long ret = -ENOENT; > + 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 > + * the symbol is found in SHY_DYNSYM, the index in that table tells us which index > + * to use in the Procedure Linking Table to instrument calls to the shared library > + * function, but locally in the binary rather than in the shared library ifself. typo: itself > + * If a binary is stripped, it may also 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 strtabidx, ndx, nr_syms; > + 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); you consistently used '%s' for binary_path, let's do that here as well > + 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)); and here > + ret = -LIBBPF_ERRNO__FORMAT; > + goto out; > + } > + nr_syms = symbols->d_size / sh.sh_entsize; > + > + for (ndx = 0; ndx < nr_syms; ndx++) { > + int curr_bind; > + GElf_Sym sym; > + > + if (!gelf_getsym(symbols, ndx, &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 && strlen(sname) > name_len && > + sname[name_len] != '@') > + continue; if both the symbol name and requested function name have @ in them, what should be the comparison rule? Shouldn't it be an exact match including '@@' and part after it? > + > + if (ret >= 0 && last_bind != -1) { if ret >= 0, last_bind can't be invalid, so let's drop the last_bind check here > + /* 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\n", > + sname, name); > + ret = -LIBBPF_ERRNO__FORMAT; > + goto out; [...]
