From f14fbe72ece11d0146d309a2f0126b058e51c7d7 Mon Sep 17 00:00:00 2001 From: Bryan Drewery Date: Fri, 27 May 2016 23:57:43 +0000 Subject: [PATCH] exec: Cease tracing if credentials will change with the new image. This also prevents tracing to a P_INEXEC process since it could race with other processes attaching to it in filemon_event_process_exec() due to the filemon_get_proc() race of incrementing ref and then locking the filemon. With the no-P_INEXEC invariant in place the p_filemon may only be the same or NULL when trying to drop it in filemon_event_process_exec(). MFC after: 2 weeks Sponsored by: EMC / Isilon Storage Division Differential Revision: https://reviews.freebsd.org/D6545 --- sys/dev/filemon/filemon.c | 13 ++++++++++--- sys/dev/filemon/filemon_wrapper.c | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/sys/dev/filemon/filemon.c b/sys/dev/filemon/filemon.c index 10f27ad1c33..1845ab9c3e3 100644 --- a/sys/dev/filemon/filemon.c +++ b/sys/dev/filemon/filemon.c @@ -89,6 +89,7 @@ MALLOC_DEFINE(M_FILEMON, "filemon", "File access monitor"); struct filemon { struct sx lock; /* Lock for this filemon. */ struct file *fp; /* Output file pointer. */ + struct ucred *cred; /* Credential of tracer. */ char fname1[MAXPATHLEN]; /* Temporary filename buffer. */ char fname2[MAXPATHLEN]; /* Temporary filename buffer. */ char msgbufr[1024]; /* Output message buffer. */ @@ -125,6 +126,8 @@ filemon_release(struct filemon *filemon) */ sx_assert(&filemon->lock, SA_UNLOCKED); + if (filemon->cred != NULL) + crfree(filemon->cred); sx_destroy(&filemon->lock); free(filemon, M_FILEMON); } @@ -308,6 +311,9 @@ filemon_attach_proc(struct filemon *filemon, struct proc *p) KASSERT((p->p_flag & P_WEXIT) == 0, ("%s: filemon %p attaching to exiting process %p", __func__, filemon, p)); + KASSERT((p->p_flag & P_INEXEC) == 0, + ("%s: filemon %p attaching to execing process %p", + __func__, filemon, p)); if (p->p_filemon == filemon) return (0); @@ -385,8 +391,8 @@ filemon_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag __unused, /* Invalidate any existing processes already set. */ filemon_untrack_processes(filemon); - error = pget(*((pid_t *)data), PGET_CANDEBUG | PGET_NOTWEXIT, - &p); + error = pget(*((pid_t *)data), + PGET_CANDEBUG | PGET_NOTWEXIT | PGET_NOTINEXEC, &p); if (error == 0) { KASSERT(p->p_filemon != filemon, ("%s: proc %p didn't untrack filemon %p", @@ -407,7 +413,7 @@ filemon_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag __unused, static int filemon_open(struct cdev *dev, int oflags __unused, int devtype __unused, - struct thread *td __unused) + struct thread *td) { int error; struct filemon *filemon; @@ -416,6 +422,7 @@ filemon_open(struct cdev *dev, int oflags __unused, int devtype __unused, M_WAITOK | M_ZERO); sx_init(&filemon->lock, "filemon"); refcount_init(&filemon->refcnt, 1); + filemon->cred = crhold(td->td_ucred); error = devfs_set_cdevpriv(filemon, filemon_dtr); if (error != 0) diff --git a/sys/dev/filemon/filemon_wrapper.c b/sys/dev/filemon/filemon_wrapper.c index e03c2376e47..64b703c196e 100644 --- a/sys/dev/filemon/filemon_wrapper.c +++ b/sys/dev/filemon/filemon_wrapper.c @@ -32,6 +32,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include @@ -112,6 +113,24 @@ filemon_event_process_exec(void *arg __unused, struct proc *p, filemon_output(filemon, filemon->msgbufr, len); + /* If the credentials changed then cease tracing. */ + if (imgp->newcred != NULL && + imgp->credential_setid && + priv_check_cred(filemon->cred, + PRIV_DEBUG_DIFFCRED, 0) != 0) { + /* + * It may have changed to NULL already, but + * will not be re-attached by anything else. + */ + if (p->p_filemon != NULL) { + KASSERT(p->p_filemon == filemon, + ("%s: proc %p didn't have expected" + " filemon %p", __func__, p, filemon)); + filemon_proc_drop(p); + } + } + + filemon_drop(filemon); } }