Jilles Tjoelker <jil...@stack.nl> wrote in <20130802152204.ga1...@stack.nl>:
ji> You can simplify the code using the fairly new pget(). This will also ji> fix the incorrect errno when the process does not exist (should be ji> [ESRCH]). ji> ji> This change is a step in the right direction but is incomplete. Although ji> the check protects currently running processes, I do not see how it ji> prevents tracing a process that gets the same PID again after the ji> original target process has exited. This not only leaks sensitive ji> information but may also prevent tracing by the legitimate owner of the ji> process (because only one filemon will write events for a process). This ji> could be fixed by setting filemon->pid = -1 in ji> filemon_wrapper_sys_exit() and not allowing P_WEXIT and zombies in ji> FILEMON_SET_PID (PGET_NOTWEXIT disallows both). An [EBUSY] when there is ji> already a filemon monitoring the process may also be useful (or writing ji> copies of the events to all attached filemons). Thank you for your comments. Can you review the attached patch? If there is no problem, I will commit this and MFC to stable branches. -- Hiroki
Index: sys/dev/filemon/filemon.c =================================================================== --- sys/dev/filemon/filemon.c (revision 253911) +++ sys/dev/filemon/filemon.c (working copy) @@ -164,13 +164,12 @@ /* Set the monitored process ID. */ case FILEMON_SET_PID: - p = pfind(*((pid_t *)data)); - if (p == NULL) - return (EINVAL); - error = p_candebug(curthread, p); - if (error == 0) + error = pget(*((pid_t *)data), PGET_CANDEBUG | PGET_NOTWEXIT, + &p); + if (error == 0) { filemon->pid = p->p_pid; - PROC_UNLOCK(p); + PROC_UNLOCK(p); + } break; default: Index: sys/dev/filemon/filemon_wrapper.c =================================================================== --- sys/dev/filemon/filemon_wrapper.c (revision 253911) +++ sys/dev/filemon/filemon_wrapper.c (working copy) @@ -574,6 +574,7 @@ (uintmax_t)now.tv_sec, (uintmax_t)now.tv_usec); filemon_output(filemon, filemon->msgbufr, len); + filemon->pid = -1; } /* Unlock the found filemon structure. */
pgpUJD4b_jbq6.pgp
Description: PGP signature