Date: Sun, 04 Aug 2013 12:15:23 +0900 (JST) From: Hiroki Sato <hrs@FreeBSD.org> To: jilles@stack.nl Cc: svn-src-head@FreeBSD.org, sjg@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r253887 - head/sys/dev/filemon Message-ID: <20130804.121523.488481502477873993.hrs@allbsd.org> In-Reply-To: <20130802152204.GA1880@stack.nl> References: <201308021444.r72EiBk2059771@svn.freebsd.org> <20130802152204.GA1880@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
----Security_Multipart0(Sun_Aug__4_12_15_23_2013_113)--
Content-Type: Multipart/Mixed;
boundary="--Next_Part(Sun_Aug__4_12_15_23_2013_653)--"
Content-Transfer-Encoding: 7bit
----Next_Part(Sun_Aug__4_12_15_23_2013_653)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Jilles Tjoelker <jilles@stack.nl> wrote
in <20130802152204.GA1880@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
----Next_Part(Sun_Aug__4_12_15_23_2013_653)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="filemon_privcheck.20130804-1.diff"
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. */
----Next_Part(Sun_Aug__4_12_15_23_2013_653)----
----Security_Multipart0(Sun_Aug__4_12_15_23_2013_113)--
Content-Type: application/pgp-signature
Content-Transfer-Encoding: 7bit
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (FreeBSD)
iEYEABECAAYFAlH9x0sACgkQTyzT2CeTzy2/EgCgwY3wB89hp+Q7oAxhFw9pgi73
Es0AnRoeXb4glizY4oW5X6ZiOFCupKTS
=eRc9
-----END PGP SIGNATURE-----
----Security_Multipart0(Sun_Aug__4_12_15_23_2013_113)----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130804.121523.488481502477873993.hrs>