pmc: Rework PROCEXEC event to support PIEs

[jrtc27]

Summary

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.

[jhb]

lib/libpmcstat/libpmcstat_process.c

92

Presumably pi_vaddr is always 0 for PIEs?

139

Presumably rtld is always PIE which is why rtldimage->pi_vaddr can be ignored here?

sys/kern/kern_exec.c

922–923

I realize these names are derived from the imgp field names, but it doesn't quite read intuitive that 'baseaddr' is 'base address of interpreter' vs if it was named something like 'interpaddr'. Even 'dynaddr' isn't fully intuitive either. Given how the variables are used in libpmc I would maybe call them 'pm_interpaddr' and 'pm_execaddr' (exec for executable). Perhaps though that doesn't quite DTRT for PDEs (or would exposing the true pm_execaddr mean you would drop the image->pi_vaddr + in libpmc)?

[jrtc27]

lib/libpmcstat/libpmcstat_process.c

92

Yes. If you have a non-zero base address for ET_DYN then the kernel will honour it.

139

In effect (otherwise the existing code wouldn't have worked; it relied on pi_entry being the offset of the entry point in order to turn entryaddr back into baseaddr). Really it's a shared library that has an entry point (I guess a static PIE in effect, even?).

sys/kern/kern_exec.c

922–923

Hm, I could change the names in PMC land... it's not just that this is the imgp name, it's also that it matches AT_BASE in the auxargs. And if you have a statically-linked binary then there is no interpreter and reloc_base will correspond to the executable.

And yes, et_dyn_addr is 0 for PDEs.

[emaste]

lib/libpmcstat/libpmcstat_process.c

69

I think we could just delete this function

sys/sys/pmc.h

6 ↗

(On Diff #120387)

Aside, we can delete the All rights reserved. if @jkoshy agrees

62 ↗

(On Diff #120387)

I wonder if we've ever incremented the patch version

[jkoshy]

sys/sys/pmc.h

6 ↗

(On Diff #120387)

No objections to removing the phrase.

[emaste]

sys/sys/pmc.h

6 ↗

(On Diff #120387)

Ok, I can pick that up before/after this lands.

I've been removing them from ones where the Foundation is the only listed copyright holder, but in general have left ones with multiple copyright holders alone for now.

[jhb]

sys/kern/kern_exec.c

922–923

Yes, the weird nature of reloc_base being the executable has been confusing to me in the kernel code as well. :) Matching AT_BASE is a decent argument though. Might be good to document what these fields are (even '/* AT_BASE */' would be a good pointer) in the header for the structure shared with userland.

[jkoshy]

lib/libpmc/pmclog.h

135–136

A new log type (say PMCLOG_TYPE_PROCEXEC_PIE and struct pmclog_ev_procexec_pie?) would be better IMHO.

This would allow pmcstat(1) to process both old and new log file formats for a couple of releases.

[jrtc27]

lib/libpmc/pmclog.h

135–136

If you have old logs you'll need the old rootfs for those logs, at which point just use the pmcstat from that rootfs

[jrtc27]

lib/libpmc/pmclog.h

135–136

Also, there are other changes which will bump the major version anyway, so it'd still be incompatible. Best to just batch everything up and break it all in one go.

[jrtc27]

• Split CONFIGURELOG and version bump out
• Comment fields in pmclog.h

[jhb]

sys/sys/pmclog.h

77

Given the other comments in this block, it might make sense to allocate a new number for the new procexec type even if the code only handles the new format. You could call the old one PMCLOG_TYPE_PROCEXEC_FBSD13 (for value 9) and add the new one here? That is a small change that doesn't disrupt the other parts of the patch and seems more consistent with the past track record here which adds new types when changing the ABI of a message.

[jrtc27]

sys/sys/pmclog.h

77

ABI breaking changes have been made to existing log types without renumbering them. From looking at the history myself, it seems that there's a difference between tweaking an existing record, which doesn't get a new number, just an ABI bump, and adding a new one that's a complete rethink of the old approach, which does get a new number. I don't really see what purpose leaving the old number reserved serves when the version number in the log gives you an unambiguous way to determine what the format is.

77

(and new number comes with new name)