Page MenuHomeFreeBSD

hwpmc: Improve re-attachment logic in pmc_process_exec()
Changes PlannedPublic

Authored by mhorne on May 26 2023, 6:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jan 13, 12:20 AM
Unknown Object (File)
Dec 21 2024, 1:55 PM
Unknown Object (File)
Dec 10 2024, 6:42 PM
Unknown Object (File)
Nov 29 2024, 11:28 AM
Unknown Object (File)
Nov 5 2024, 2:55 PM
Unknown Object (File)
Oct 5 2024, 8:40 PM
Unknown Object (File)
Sep 26 2024, 5:56 PM
Unknown Object (File)
Sep 23 2024, 5:17 PM
Subscribers

Details

Reviewers
jkoshy
emaste
gnn
markj
Group Reviewers
pmc
Summary

When a process transforms after an execve() call, we call into a hwpmc
hook to handle a few things. In particular, if the process had PMCs
attached to it, we need to evaluate whether the owning process has
sufficient privilege to re-attach its PMCs to the (new) target process.
In particular, we handle the case where the process credentials have
changed due to setuid on the exec'ed program.

Previously, we were evaluating this using a set of hand-rolled
credential checks, that partially duplicate the logic found in
p_candebug(). In the initial PMC attachment path (PMC_OP_PMCATTACH via
syscall), we call p_candebug() to determine if the thread owning the PMC
has sufficient privileges to attach it to the target process. So, change
pmc_process_exec() to use p_candebug() for the re-attachment checks as
well.

As a result, pmc_can_attach() is removed completely, and so is the
pm_credentialschanged member of struct pmckern_procexec.

This change ends up handling an additional case where the exec'ed
process now has the 'P2_NOTRACE' flag set, and re-attachment will not
proceed. This is a niche edge-case, but demonstrates why it is more
appropriate to use the core p_candebug() routine.

Some additional cleanup: specify PMC_FLAG_REMOVE for the call to the
detach routine; this will handle ref counting and process descriptor
cleanup for us. It will also clear the P_HWPMC flag when appropriate,
where before seem to have been leaking this.

Test Plan

I will verify this behaves as expected via PMCDBG trace entries.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 51776
Build 48667: arc lint + arc unit

Event Timeline

sys/dev/hwpmc/hwpmc_mod.c
1030–1031

for me the can_attach logic in each of these blocks is a little awkward, maybe a goto exit would make it a bit more clear?

1031

does this match?

There are further improvements to be made here, will update after some investigation.

sys/dev/hwpmc/hwpmc_mod.c
1031

It does not, my bad!

Rework the change; remove pmc_can_attach() entirely in favor of p_candebug().

mhorne retitled this revision from hwpmc: fix sense of pmc_can_attach() to hwpmc: Improve re-attachment logic in pmc_process_exec().May 30 2023, 2:47 PM
mhorne edited the summary of this revision. (Show Details)
mhorne edited the test plan for this revision. (Show Details)
mhorne added a reviewer: markj.
sys/dev/hwpmc/hwpmc_mod.c
1367

Note: now, all three callers of pmc_detach_one_process() pass the same flag. We could just assume this behaviour and remove the parameter, but I have not made this change.

sys/dev/hwpmc/hwpmc_mod.c
1355

Note that P2_NOTRACE specifically inhibits attaching with ptrace(2); it's ignored if the debugger has root privileges. Since hwpmc always(?) requires root privileges, P2_NOTRACE has no effect here. It similarly doesn't restrict dtrace.

1366

This requires p to be locked, but I can't see where that happens.

sys/dev/hwpmc/hwpmc_mod.c
1355

Root privileges are not required to use process-mode PMCs, so any user can perform e.g. pmcstat -p instructions -t $pid for a process they own. So in this case, pmc will respect the flag.

It is perhaps not that important to note P2_NOTRACE in the comment.

1366

Ah okay, I will fix that. That is missing from the man page.

Ugh, I did not test it properly, and I think this version is totally broken...