Date: Tue, 30 May 2023 23:37:50 GMT From: Jessica Clarke <jrtc27@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 94426d21bf62 - main - pmc: Rework PROCEXEC event to support PIEs Message-ID: <202305302337.34UNbotC056897@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by jrtc27: URL: https://cgit.FreeBSD.org/src/commit/?id=94426d21bf62f2b36dc9b556ab27c401a412a026 commit 94426d21bf62f2b36dc9b556ab27c401a412a026 Author: Jessica Clarke <jrtc27@FreeBSD.org> AuthorDate: 2023-05-30 23:20:36 +0000 Commit: Jessica Clarke <jrtc27@FreeBSD.org> CommitDate: 2023-05-30 23:20:36 +0000 pmc: Rework PROCEXEC event to support PIEs Currently the PROCEXEC event only reports a single address, entryaddr, which is the entry point of the interpreter in the typical dynamic case, and used solely to calculate the base address of the interpreter. For PDEs this is fine, since the base address is known from the program headers, but for PIEs the base address varies at run time based on where the kernel chooses to load it, and so pmcstat has no way of knowing the real address ranges for the executable. This was less of an issue in the past since PIEs were rare, but now they're on by default on 64-bit architectures it's more of a problem. To solve this, pass through what was picked for et_dyn_addr by the kernel, and use that as the offset for the executable's start address just as is done for everything in the kernel. Since we're changing this interface, sanitise the way we determine the interpreter's base address by passing it through directly rather than indirectly via the entry point and having to subtract off whatever the ELF header's e_entry is (and anything that wants the entry point in future can still add that back on as needed; this merely changes the interface to directly provide the underlying variables involved). This will be followed up by a bump to the pmc major version. Reviewed by: jhb Differential Revision: https://reviews.freebsd.org/D39595 --- lib/libpmc/libpmc_json.cc | 7 +++++-- lib/libpmc/pmclog.c | 3 ++- lib/libpmc/pmclog.h | 3 ++- lib/libpmcstat/libpmcstat.h | 6 +++--- lib/libpmcstat/libpmcstat_logging.c | 4 ++-- lib/libpmcstat/libpmcstat_process.c | 32 ++++++++++++++++---------------- sys/dev/hwpmc/hwpmc_logging.c | 9 +++++---- sys/dev/hwpmc/hwpmc_mod.c | 7 ++++--- sys/kern/kern_exec.c | 3 ++- sys/sys/pmckern.h | 3 ++- sys/sys/pmclog.h | 7 +++++-- usr.sbin/pmcstat/pmcstat_log.c | 5 +++-- 12 files changed, 51 insertions(+), 38 deletions(-) diff --git a/lib/libpmc/libpmc_json.cc b/lib/libpmc/libpmc_json.cc index 2e9857ca98a8..76c5a02732ca 100644 --- a/lib/libpmc/libpmc_json.cc +++ b/lib/libpmc/libpmc_json.cc @@ -163,9 +163,12 @@ procexec_to_json(struct pmclog_ev *ev) startent = startentry(ev); snprintf(eventbuf, sizeof(eventbuf), "%s, \"pmcid\": \"0x%08x\", \"pid\": \"%d\", " - "\"start\": \"0x%016jx\", \"pathname\": \"%s\"}\n", + "\"base\": \"0x%016jx\", \"dyn\": \"0x%016jx\", " + "\"pathname\": \"%s\"}\n", startent.c_str(), ev->pl_u.pl_x.pl_pmcid, ev->pl_u.pl_x.pl_pid, - (uintmax_t)ev->pl_u.pl_x.pl_entryaddr, ev->pl_u.pl_x.pl_pathname); + (uintmax_t)ev->pl_u.pl_x.pl_baseaddr, + (uintmax_t)ev->pl_u.pl_x.pl_dynaddr, + ev->pl_u.pl_x.pl_pathname); return string(eventbuf); } diff --git a/lib/libpmc/pmclog.c b/lib/libpmc/pmclog.c index babcdc3c8d0d..0db91cf51bc2 100644 --- a/lib/libpmc/pmclog.c +++ b/lib/libpmc/pmclog.c @@ -393,7 +393,8 @@ pmclog_get_event(void *cookie, char **data, ssize_t *len, PMCLOG_GET_PATHLEN(pathlen,evlen,pmclog_procexec); PMCLOG_READ32(le,ev->pl_u.pl_x.pl_pid); PMCLOG_READ32(le,ev->pl_u.pl_x.pl_pmcid); - PMCLOG_READADDR(le,ev->pl_u.pl_x.pl_entryaddr); + PMCLOG_READADDR(le,ev->pl_u.pl_x.pl_baseaddr); + PMCLOG_READADDR(le,ev->pl_u.pl_x.pl_dynaddr); PMCLOG_READSTRING(le,ev->pl_u.pl_x.pl_pathname,pathlen); break; case PMCLOG_TYPE_PROCEXIT: diff --git a/lib/libpmc/pmclog.h b/lib/libpmc/pmclog.h index c81246b168eb..c2973e9a365a 100644 --- a/lib/libpmc/pmclog.h +++ b/lib/libpmc/pmclog.h @@ -132,7 +132,8 @@ struct pmclog_ev_proccreate { struct pmclog_ev_procexec { pid_t pl_pid; pmc_id_t pl_pmcid; - uintfptr_t pl_entryaddr; + uintptr_t pl_baseaddr; + uintptr_t pl_dynaddr; char pl_pathname[PATH_MAX]; }; diff --git a/lib/libpmcstat/libpmcstat.h b/lib/libpmcstat/libpmcstat.h index 07d82d4d0e57..87bd3a185f40 100644 --- a/lib/libpmcstat/libpmcstat.h +++ b/lib/libpmcstat/libpmcstat.h @@ -336,7 +336,7 @@ struct pmcstat_image * int pmcstat_string_lookup_hash(pmcstat_interned_string _is); void pmcstat_process_elf_exec(struct pmcstat_process *_pp, - struct pmcstat_image *_image, uintfptr_t _entryaddr, + struct pmcstat_image *_image, uintptr_t _baseaddr, uintptr_t _dynaddr, struct pmcstat_args *args, struct pmc_plugins *plugins, struct pmcstat_stats *pmcstat_stats); @@ -344,9 +344,9 @@ void pmcstat_image_link(struct pmcstat_process *_pp, struct pmcstat_image *_i, uintfptr_t _lpc); void pmcstat_process_aout_exec(struct pmcstat_process *_pp, - struct pmcstat_image *_image, uintfptr_t _entryaddr); + struct pmcstat_image *_image, uintptr_t _baseaddr); void pmcstat_process_exec(struct pmcstat_process *_pp, - pmcstat_interned_string _path, uintfptr_t _entryaddr, + pmcstat_interned_string _path, uintptr_t _baseaddr, uintptr_t _dynaddr, struct pmcstat_args *args, struct pmc_plugins *plugins, struct pmcstat_stats *pmcstat_stats); void pmcstat_image_determine_type(struct pmcstat_image *_image, struct pmcstat_args *args); diff --git a/lib/libpmcstat/libpmcstat_logging.c b/lib/libpmcstat/libpmcstat_logging.c index 42054e636b4b..b41e93b4f729 100644 --- a/lib/libpmcstat/libpmcstat_logging.c +++ b/lib/libpmcstat/libpmcstat_logging.c @@ -353,8 +353,8 @@ pmcstat_analyze_log(struct pmcstat_args *args, ev.pl_u.pl_x.pl_pathname); assert(image_path != NULL); pmcstat_process_exec(pp, image_path, - ev.pl_u.pl_x.pl_entryaddr, args, - plugins, pmcstat_stats); + ev.pl_u.pl_x.pl_baseaddr, ev.pl_u.pl_x.pl_dynaddr, + args, plugins, pmcstat_stats); break; case PMCLOG_TYPE_PROCEXIT: diff --git a/lib/libpmcstat/libpmcstat_process.c b/lib/libpmcstat/libpmcstat_process.c index 147eff9ab23e..4d710eac2f58 100644 --- a/lib/libpmcstat/libpmcstat_process.c +++ b/lib/libpmcstat/libpmcstat_process.c @@ -61,11 +61,11 @@ __FBSDID("$FreeBSD$"); void pmcstat_process_aout_exec(struct pmcstat_process *pp, - struct pmcstat_image *image, uintfptr_t entryaddr) + struct pmcstat_image *image, uintptr_t baseaddr) { (void) pp; (void) image; - (void) entryaddr; + (void) baseaddr; /* TODO Implement a.out handling */ } @@ -75,18 +75,21 @@ pmcstat_process_aout_exec(struct pmcstat_process *pp, void pmcstat_process_elf_exec(struct pmcstat_process *pp, - struct pmcstat_image *image, uintfptr_t entryaddr, + struct pmcstat_image *image, uintptr_t baseaddr, uintptr_t dynaddr, struct pmcstat_args *args, struct pmc_plugins *plugins, struct pmcstat_stats *pmcstat_stats) { - uintmax_t libstart; struct pmcstat_image *rtldimage; assert(image->pi_type == PMCSTAT_IMAGE_ELF32 || image->pi_type == PMCSTAT_IMAGE_ELF64); - /* Create a map entry for the base executable. */ - pmcstat_image_link(pp, image, image->pi_vaddr); + /* + * The exact address where the executable gets mapped in will vary for + * PIEs. The dynamic address recorded at process exec time corresponds + * to the address where the executable's file object had been mapped to. + */ + pmcstat_image_link(pp, image, image->pi_vaddr + dynaddr); /* * For dynamically linked executables we need to determine @@ -105,16 +108,14 @@ pmcstat_process_elf_exec(struct pmcstat_process *pp, * [ TEXT DATA BSS HEAP -->*RTLD SHLIBS <--STACK] * ^ ^ * 0 VM_MAXUSER_ADDRESS - * * The exact address where the loader gets mapped in * will vary according to the size of the executable * and the limits on the size of the process'es data - * segment at the time of exec(). The entry address + * segment at the time of exec(). The base address * recorded at process exec time corresponds to the - * 'start' address inside the dynamic linker. From - * this we can figure out the address where the - * runtime loader's file object had been mapped to. + * address where the runtime loader's file object had + * been mapped to. */ rtldimage = pmcstat_image_from_path(image->pi_dynlinkerpath, 0, args, plugins); @@ -135,8 +136,7 @@ pmcstat_process_elf_exec(struct pmcstat_process *pp, return; } - libstart = entryaddr - rtldimage->pi_entry; - pmcstat_image_link(pp, rtldimage, libstart); + pmcstat_image_link(pp, rtldimage, baseaddr); } } @@ -146,7 +146,7 @@ pmcstat_process_elf_exec(struct pmcstat_process *pp, void pmcstat_process_exec(struct pmcstat_process *pp, - pmcstat_interned_string path, uintfptr_t entryaddr, + pmcstat_interned_string path, uintptr_t baseaddr, uintptr_t dynaddr, struct pmcstat_args *args, struct pmc_plugins *plugins, struct pmcstat_stats *pmcstat_stats) { @@ -167,13 +167,13 @@ pmcstat_process_exec(struct pmcstat_process *pp, case PMCSTAT_IMAGE_ELF32: case PMCSTAT_IMAGE_ELF64: pmcstat_stats->ps_exec_elf++; - pmcstat_process_elf_exec(pp, image, entryaddr, + pmcstat_process_elf_exec(pp, image, baseaddr, dynaddr, args, plugins, pmcstat_stats); break; case PMCSTAT_IMAGE_AOUT: pmcstat_stats->ps_exec_aout++; - pmcstat_process_aout_exec(pp, image, entryaddr); + pmcstat_process_aout_exec(pp, image, baseaddr); break; case PMCSTAT_IMAGE_INDETERMINABLE: diff --git a/sys/dev/hwpmc/hwpmc_logging.c b/sys/dev/hwpmc/hwpmc_logging.c index 02f2c1c383e2..00ecd361a866 100644 --- a/sys/dev/hwpmc/hwpmc_logging.c +++ b/sys/dev/hwpmc/hwpmc_logging.c @@ -198,9 +198,9 @@ CTASSERT(offsetof(struct pmclog_pmcattach,pl_pathname) == 5*4 + TSDELTA); CTASSERT(sizeof(struct pmclog_pmcdetach) == 5*4 + TSDELTA); CTASSERT(sizeof(struct pmclog_proccsw) == 7*4 + 8 + TSDELTA); CTASSERT(sizeof(struct pmclog_procexec) == 5*4 + PATH_MAX + - sizeof(uintfptr_t) + TSDELTA); + 2*sizeof(uintptr_t) + TSDELTA); CTASSERT(offsetof(struct pmclog_procexec,pl_pathname) == 5*4 + TSDELTA + - sizeof(uintfptr_t)); + 2*sizeof(uintptr_t)); CTASSERT(sizeof(struct pmclog_procexit) == 5*4 + 8 + TSDELTA); CTASSERT(sizeof(struct pmclog_procfork) == 5*4 + TSDELTA); CTASSERT(sizeof(struct pmclog_sysexit) == 6*4); @@ -1096,7 +1096,7 @@ pmclog_process_proccsw(struct pmc *pm, struct pmc_process *pp, pmc_value_t v, st void pmclog_process_procexec(struct pmc_owner *po, pmc_id_t pmid, pid_t pid, - uintfptr_t startaddr, char *path) + uintptr_t baseaddr, uintptr_t dynaddr, char *path) { int pathlen, recordlen; @@ -1107,7 +1107,8 @@ pmclog_process_procexec(struct pmc_owner *po, pmc_id_t pmid, pid_t pid, PMCLOG_RESERVE(po, PMCLOG_TYPE_PROCEXEC, recordlen); PMCLOG_EMIT32(pid); PMCLOG_EMIT32(pmid); - PMCLOG_EMITADDR(startaddr); + PMCLOG_EMITADDR(baseaddr); + PMCLOG_EMITADDR(dynaddr); PMCLOG_EMITSTRING(path,pathlen); PMCLOG_DESPATCH_SYNC(po); } diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c index 830e73941fb6..b3cf309fb74e 100644 --- a/sys/dev/hwpmc/hwpmc_mod.c +++ b/sys/dev/hwpmc/hwpmc_mod.c @@ -2126,7 +2126,8 @@ pmc_hook_handler(struct thread *td, int function, void *arg) CK_LIST_FOREACH(po, &pmc_ss_owners, po_ssnext) if (po->po_flags & PMC_PO_OWNS_LOGFILE) pmclog_process_procexec(po, PMC_ID_INVALID, - p->p_pid, pk->pm_entryaddr, fullpath); + p->p_pid, pk->pm_baseaddr, pk->pm_dynaddr, + fullpath); PMC_EPOCH_EXIT(); PROC_LOCK(p); @@ -2170,8 +2171,8 @@ pmc_hook_handler(struct thread *td, int function, void *arg) if (po->po_sscount == 0 && po->po_flags & PMC_PO_OWNS_LOGFILE) pmclog_process_procexec(po, pm->pm_id, - p->p_pid, pk->pm_entryaddr, - fullpath); + p->p_pid, pk->pm_baseaddr, + pk->pm_dynaddr, fullpath); } if (freepath) diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 14aac3f374d2..a779aa11b4c3 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -919,7 +919,8 @@ interpret: if (PMC_SYSTEM_SAMPLING_ACTIVE() || PMC_PROC_IS_USING_PMCS(p)) { VOP_UNLOCK(imgp->vp); pe.pm_credentialschanged = credential_changing; - pe.pm_entryaddr = imgp->entry_addr; + pe.pm_baseaddr = imgp->reloc_base; + pe.pm_dynaddr = imgp->et_dyn_addr; PMC_CALL_HOOK_X(td, PMC_FN_PROCESS_EXEC, (void *) &pe); vn_lock(imgp->vp, LK_SHARED | LK_RETRY); diff --git a/sys/sys/pmckern.h b/sys/sys/pmckern.h index 7012b0bc9de4..93e772c24563 100644 --- a/sys/sys/pmckern.h +++ b/sys/sys/pmckern.h @@ -76,7 +76,8 @@ typedef enum ring_type { struct pmckern_procexec { int pm_credentialschanged; - uintfptr_t pm_entryaddr; + uintptr_t pm_baseaddr; + uintptr_t pm_dynaddr; }; struct pmckern_map_in { diff --git a/sys/sys/pmclog.h b/sys/sys/pmclog.h index 0ce2a29263bf..3659b2505daa 100644 --- a/sys/sys/pmclog.h +++ b/sys/sys/pmclog.h @@ -202,7 +202,10 @@ struct pmclog_procexec { PMCLOG_ENTRY_HEADER uint32_t pl_pid; uint32_t pl_pmcid; - uintfptr_t pl_start; /* keep 8 byte aligned */ + /* keep 8 byte aligned */ + uintptr_t pl_base; /* AT_BASE */ + /* keep 8 byte aligned */ + uintptr_t pl_dyn; /* PIE load base */ char pl_pathname[PATH_MAX]; } __packed; @@ -314,7 +317,7 @@ void pmclog_process_pmcdetach(struct pmc *_pm, pid_t _pid); void pmclog_process_proccsw(struct pmc *_pm, struct pmc_process *_pp, pmc_value_t _v, struct thread *); void pmclog_process_procexec(struct pmc_owner *_po, pmc_id_t _pmid, pid_t _pid, - uintfptr_t _startaddr, char *_path); + uintfptr_t _baseaddr, uintptr_t _dynaddr, char *_path); void pmclog_process_procexit(struct pmc *_pm, struct pmc_process *_pp); void pmclog_process_procfork(struct pmc_owner *_po, pid_t _oldpid, pid_t _newpid); void pmclog_process_sysexit(struct pmc_owner *_po, pid_t _pid); diff --git a/usr.sbin/pmcstat/pmcstat_log.c b/usr.sbin/pmcstat/pmcstat_log.c index 3f764da964cd..7ff5d032fc99 100644 --- a/usr.sbin/pmcstat/pmcstat_log.c +++ b/usr.sbin/pmcstat/pmcstat_log.c @@ -459,10 +459,11 @@ pmcstat_print_log(void) ev.pl_u.pl_pc.pl_pcomm); break; case PMCLOG_TYPE_PROCEXEC: - PMCSTAT_PRINT_ENTRY("exec","0x%x %d %p \"%s\"", + PMCSTAT_PRINT_ENTRY("exec","0x%x %d %p %p \"%s\"", ev.pl_u.pl_x.pl_pmcid, ev.pl_u.pl_x.pl_pid, - (void *) ev.pl_u.pl_x.pl_entryaddr, + (void *)ev.pl_u.pl_x.pl_baseaddr, + (void *)ev.pl_u.pl_x.pl_dynaddr, ev.pl_u.pl_x.pl_pathname); break; case PMCLOG_TYPE_PROCEXIT:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202305302337.34UNbotC056897>