From owner-svn-src-head@FreeBSD.ORG Sun Aug 4 03:16:54 2013 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id A55CAA28; Sun, 4 Aug 2013 03:16:54 +0000 (UTC) (envelope-from hrs@FreeBSD.org) Received: from mail.allbsd.org (gatekeeper.allbsd.org [IPv6:2001:2f0:104:e001::32]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 600792928; Sun, 4 Aug 2013 03:16:53 +0000 (UTC) Received: from alph.d.allbsd.org (p2049-ipbf1102funabasi.chiba.ocn.ne.jp [122.26.101.49]) (authenticated bits=128) by mail.allbsd.org (8.14.5/8.14.5) with ESMTP id r743GYxt092725 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 4 Aug 2013 12:16:44 +0900 (JST) (envelope-from hrs@FreeBSD.org) Received: from localhost (localhost [IPv6:::1]) (authenticated bits=0) by alph.d.allbsd.org (8.14.5/8.14.5) with ESMTP id r743GV8X094817; Sun, 4 Aug 2013 12:16:34 +0900 (JST) (envelope-from hrs@FreeBSD.org) Date: Sun, 04 Aug 2013 12:15:23 +0900 (JST) Message-Id: <20130804.121523.488481502477873993.hrs@allbsd.org> To: jilles@stack.nl Subject: Re: svn commit: r253887 - head/sys/dev/filemon From: Hiroki Sato In-Reply-To: <20130802152204.GA1880@stack.nl> References: <201308021444.r72EiBk2059771@svn.freebsd.org> <20130802152204.GA1880@stack.nl> X-PGPkey-fingerprint: BDB3 443F A5DD B3D0 A530 FFD7 4F2C D3D8 2793 CF2D X-Mailer: Mew version 6.5 on Emacs 24.3 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Multipart/Signed; protocol="application/pgp-signature"; micalg=pgp-sha1; boundary="--Security_Multipart0(Sun_Aug__4_12_15_23_2013_113)--" Content-Transfer-Encoding: 7bit X-Virus-Scanned: clamav-milter 0.97.4 at gatekeeper.allbsd.org X-Virus-Status: Clean X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (mail.allbsd.org [133.31.130.32]); Sun, 04 Aug 2013 12:16:45 +0900 (JST) X-Spam-Status: No, score=-90.6 required=13.0 tests=CONTENT_TYPE_PRESENT, DIRECTOCNDYN,DYN_PBL,RCVD_IN_PBL,SPF_SOFTFAIL,USER_IN_WHITELIST autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on gatekeeper.allbsd.org Cc: svn-src-head@FreeBSD.org, sjg@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Aug 2013 03:16:54 -0000 ----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 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)----