On Wed, Mar 1, 2023 at 10:40 AM Daniel Müller <deso@xxxxxxxxxx> wrote: > > This change adds support for attaching uprobes to shared objects located > in APKs, which is relevant for Android systems where various libraries > may reside in APKs. To make that happen, we extend the syntax for the > "binary path" argument to attach to with that supported by various > Android tools: > <archive>!/<binary-in-archive> > > For example: > /system/app/test-app/test-app.apk!/lib/arm64-v8a/libc++_shared.so > > APKs need to be specified via full path, i.e., we do not attempt to > resolve mere file names by searching system directories. > > We cannot currently test this functionality end-to-end in an automated > fashion, because it relies on an Android system being present, but there > is no support for that in CI. I have tested the functionality manually, > by creating a libbpf program containing a uretprobe, attaching it to a > function inside a shared object inside an APK, and verifying the sanity > of the returned values. > > Signed-off-by: Daniel Müller <deso@xxxxxxxxxx> > --- > tools/lib/bpf/libbpf.c | 92 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 85 insertions(+), 7 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 4543e9..e6b99a 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -53,6 +53,7 @@ > #include "libbpf_internal.h" > #include "hashmap.h" > #include "bpf_gen_internal.h" > +#include "zip.h" > > #ifndef BPF_FS_MAGIC > #define BPF_FS_MAGIC 0xcafe4a11 > @@ -10702,6 +10703,69 @@ static long elf_find_func_offset_from_file(const char *binary_path, const char * > return ret; > } > > +/* Find offset of function name in archive specified by path. Currently > + * supported are .zip files that do not compress their contents, as used on > + * Android in the form of APKs, for example. "file_name" is the name of the ELF > + * file inside the archive. "func_name" matches symbol name or name@@LIB for > + * library functions. > + * > + * An overview of the APK format specifically provided here: > + * https://en.wikipedia.org/w/index.php?title=Apk_(file_format)&oldid=1139099120#Package_contents > + */ > +static long elf_find_func_offset_from_archive(const char *archive_path, const char *file_name, > + const char *func_name) > +{ > + struct zip_archive *archive; > + struct zip_entry entry; > + long ret = -ENOENT; > + int err; > + Elf *elf; > + > + archive = zip_archive_open(archive_path); > + if (IS_ERR(archive)) { Unfortunately, this won't work with the libbpf_err_ptr() approach that you used inside zip_archive_open(). Since libbpf v1.0 libbpf_err_ptr() will return NULL on error (and so this IS_ERR() check will always be false, and subsequent PTR_ERR() would be returning 0) and only set errno to actual error. This was meant to be used mostly for user-facing APIs. Given zip_archive_open() is internal, explicit PTR_ERR() use as you do below makes most sense. Please update and respin. > + pr_warn("zip: failed to open %s: %ld\n", archive_path, PTR_ERR(archive)); > + return PTR_ERR(archive); err = PTR_ERR(archive); and use err in pr_warn() and return? and it's not clear why you need both ret and err, it should be fine to just use ret (long vs int doesn't hurt error propagation) > + } > + > + err = zip_archive_find_entry(archive, file_name, &entry); > + if (err) { > + pr_warn("zip: could not find archive member %s in %s: %d\n", file_name, > + archive_path, err); > + ret = err; > + goto out; > + } > + pr_debug("zip: found entry for %s in %s at 0x%lx\n", file_name, archive_path, > + (unsigned long)entry.data_offset); > + [...]