git: cbf3fe8549b6 - main - hwpmc: Fix the execve handler

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Tue, 25 Aug 2026 15:59:19 UTC
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=cbf3fe8549b68745bddd6c6ee4a0f3233eacbe85

commit cbf3fe8549b68745bddd6c6ee4a0f3233eacbe85
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-08-24 14:57:16 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-08-25 15:46:23 +0000

    hwpmc: Fix the execve handler
    
    When a process execve()s, pmc_process_exec() is supposed to evaluate
    whether the new image is setuid/setgid and if so, whether to detach
    PMCs.  This was handled by pmc_can_attach(), which is effectively an
    open-coded copy of cr_xids_subset().
    
    Unfortunately, the test of the result of this function was inverted,
    with the result that we'd detach PMCs only if the predicate said it was
    okay to do so.  It appears the bug has always been there; it seems the
    intent was to return 0 on "success", i.e., it is okay to attach the
    PMCs, much like p_candebug().  Commits 1c3c698ba4c4 and 1c40b15971f0
    obscured this a bit.
    
    I think this check is trying to be too clever.  Let's make it simpler:
    simply do not attach PMCs unless the owner is privileged.  This is how,
    e.g., ktrace works.  I do not think it's worth trying to be more
    sophisticated than this unless we can generalize the policy in a way
    that's applicable to other subsystems.
    
    Also fix a bug at the end of pmc_process_exec():
    pmc_detach_one_process() will call pmc_remove_process_descriptor() for
    us.
    
    Approved by:    so
    Security:       FreeBSD-SA-26:56.hwpmc
    Security:       CVE-2026-58089
    Reported by:    netchild
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D59102
---
 sys/dev/hwpmc/hwpmc_mod.c | 72 ++++++++---------------------------------------
 sys/kern/kern_exec.c      |  2 +-
 2 files changed, 13 insertions(+), 61 deletions(-)

diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c
index bfa8a217235e..6c405d53486b 100644
--- a/sys/dev/hwpmc/hwpmc_mod.c
+++ b/sys/dev/hwpmc/hwpmc_mod.c
@@ -218,7 +218,6 @@ static int	pmc_attach_one_process(struct proc *p, struct pmc *pm);
 static bool	pmc_can_allocate_row(int ri, enum pmc_mode mode);
 static bool	pmc_can_allocate_rowindex(struct proc *p, unsigned int ri,
     int cpu);
-static bool	pmc_can_attach(struct pmc *pm, struct proc *p);
 static void	pmc_capture_user_callchain(int cpu, int soft,
     struct trapframe *tf);
 static void	pmc_cleanup(void);
@@ -1033,60 +1032,6 @@ pmc_unlink_target_process(struct pmc *pm, struct pmc_process *pp)
 	}
 }
 
-/*
- * Check if PMC 'pm' may be attached to target process 't'.
- */
-
-static bool
-pmc_can_attach(struct pmc *pm, struct proc *t)
-{
-	struct proc *o;		/* pmc owner */
-	struct ucred *oc, *tc;	/* owner, target credentials */
-	bool decline_attach;
-
-	/*
-	 * A PMC's owner can always attach that PMC to itself.
-	 */
-
-	if ((o = pm->pm_owner->po_owner) == t)
-		return (true);
-
-	PROC_LOCK(o);
-	oc = o->p_ucred;
-	crhold(oc);
-	PROC_UNLOCK(o);
-
-	PROC_LOCK(t);
-	tc = t->p_ucred;
-	crhold(tc);
-	PROC_UNLOCK(t);
-
-	/*
-	 * The effective uid of the PMC owner should match at least one
-	 * of the {effective,real,saved} uids of the target process.
-	 */
-
-	decline_attach = oc->cr_uid != tc->cr_uid &&
-	    oc->cr_uid != tc->cr_svuid &&
-	    oc->cr_uid != tc->cr_ruid;
-
-	/*
-	 * Every one of the target's group ids, must be in the owner's
-	 * group list.
-	 */
-	for (int i = 0; !decline_attach && i < tc->cr_ngroups; i++)
-		decline_attach = !groupmember(tc->cr_groups[i], oc);
-	if (!decline_attach)
-		decline_attach = !groupmember(tc->cr_gid, oc) ||
-		    !groupmember(tc->cr_rgid, oc) ||
-		    !groupmember(tc->cr_svgid, oc);
-
-	crfree(tc);
-	crfree(oc);
-
-	return (!decline_attach);
-}
-
 /*
  * Attach a process to a PMC.
  */
@@ -1450,10 +1395,19 @@ pmc_process_exec(struct thread *td, struct pmckern_procexec *pk)
 	 */
 	for (ri = 0; ri < md->pmd_npmc; ri++) {
 		if ((pm = pp->pp_pmcs[ri].pp_pmc) != NULL) {
-			if (pmc_can_attach(pm, td->td_proc)) {
+			struct proc *owner;
+			struct ucred *cred;
+
+			owner = pm->pm_owner->po_owner;
+			PROC_LOCK(owner);
+			cred = crhold(owner->p_ucred);
+			PROC_UNLOCK(owner);
+
+			if (priv_check_cred(cred, PRIV_DEBUG_DIFFCRED) != 0)
 				pmc_detach_one_process(td->td_proc, pm,
 				    PMC_FLAG_NONE);
-			}
+
+			crfree(cred);
 		}
 	}
 
@@ -1466,10 +1420,8 @@ pmc_process_exec(struct thread *td, struct pmckern_procexec *pk)
 	 * PMCs, we can remove the process entry and free
 	 * up space.
 	 */
-	if (pp->pp_refcnt == 0) {
-		pmc_remove_process_descriptor(pp);
+	if (pp->pp_refcnt == 0)
 		pmc_destroy_process_descriptor(pp);
-	}
 }
 
 /*
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
index 352170d4f6f0..7871ac9b86fa 100644
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -1037,7 +1037,7 @@ interpret:
 	 */
 	if (PMC_SYSTEM_SAMPLING_ACTIVE() || PMC_PROC_IS_USING_PMCS(p)) {
 		VOP_UNLOCK(imgp->vp);
-		pe.pm_credentialschanged = credential_changing;
+		pe.pm_credentialschanged = imgp->credential_setid;
 		pe.pm_baseaddr = imgp->reloc_base;
 		pe.pm_dynaddr = imgp->et_dyn_addr;