From owner-svn-src-head@FreeBSD.ORG Thu Dec 19 06:20:20 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 ESMTPS id 37172671; Thu, 19 Dec 2013 06:20:20 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id C9ED61EB9; Thu, 19 Dec 2013 06:20:19 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.7/8.14.7) with ESMTP id rBJ6K8R2059052; Thu, 19 Dec 2013 08:20:08 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.8.3 kib.kiev.ua rBJ6K8R2059052 Received: (from kostik@localhost) by tom.home (8.14.7/8.14.7/Submit) id rBJ6K8F9059051; Thu, 19 Dec 2013 08:20:08 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 19 Dec 2013 08:20:08 +0200 From: Konstantin Belousov To: Mateusz Guzik Subject: Re: svn commit: r259407 - head/sys/kern Message-ID: <20131219062008.GK59496@kib.kiev.ua> References: <201312150411.rBF4Bhtg018852@svn.freebsd.org> <201312171141.49251.jhb@freebsd.org> <20131217181745.GB7535@dft-labs.eu> <201312171434.01345.jhb@freebsd.org> <20131219002824.GA5664@dft-labs.eu> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Bk/e7+bhc7oa9AZn" Content-Disposition: inline In-Reply-To: <20131219002824.GA5664@dft-labs.eu> User-Agent: Mutt/1.5.22 (2013-10-16) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik , John Baldwin X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 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: Thu, 19 Dec 2013 06:20:20 -0000 --Bk/e7+bhc7oa9AZn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 19, 2013 at 01:28:24AM +0100, Mateusz Guzik wrote: > That being said, instead of reverting the change (which would leave other= field > with similar issue in place) I propose adding the following: > --- a/sys/kern/kern_exit.c > +++ b/sys/kern/kern_exit.c > @@ -220,6 +220,12 @@ exit1(struct thread *td, int rv) > =20 > p->p_xstat =3D rv; /* Let event handler change exit status= */ > PROC_UNLOCK(p); > + > + /* > + * Some fields below are freed without having the proc locked, ch= eck > + * for P_WEXIT before accessing to make sure it is safe. > + */ > + >=20 > Which should make it clear. >=20 > But again, this is a cosmetic change and I have no strong opinion either > way. If you are still unconvinced I'm happy to revert it later. I think adding a comment is fine, but besides the comment you propose for describing what does the code do in the exit path, it is more important to put the same comment into the struct proc description. In other words, fields should be annotated to indicate which accesses require gating on exiting state. I think something similar should be done for the execing state. The kern_proc.c sysctls are already full of this knowledge, and since you are making it explicit in one case, doing the full pass makes sense. --Bk/e7+bhc7oa9AZn Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iQIcBAEBAgAGBQJSspAXAAoJEJDCuSvBvK1BqegQAKXPuyActrnrqIkjsZeswJmx 8l3IifWy8jdi37fAoPHba43TsysQk0AMIAgeUyERmM4nMOYBNUd2GlOkXLP7ixF2 2JXFNbJpPq672WrF+RvfyHUx+JtcDAQ3e76gElDrJnXBOvw7hR+oHSqJcLBQP14V 7DkCjYRV70XQxYgCeqyLdv1H5oMke6skfcrgMATarmmqt0XCgnKzAVbgnLK8QbJe 8dY4HvlofvVZRaCg6JiiurbFxT8Treg5vgNpdRS0sBP0YwF55TpQKvynnHARRhsW 8GsFCDI8sz7INXLNnb3xSq3gcu9VUBnk9l8UlWa8PKxga/CqrQupdLPuN7whRRUE sET38I/1znKhWixuFdk/Cc1Jhc36gJGUXGfy7tfexisZ+7+PTQ7ceyTIXf9nSfUj 7kMV5RHU4oIH0nbmQjI3nyTQajYv2qmyRriWaHGoyuid/3UO1uj5B2WrHn3FBdRa HJ7okbFLDznEhwxU2So7AKZYBCPCrGRoekZj/2TiPa9OMBC4y01akSYXOrSFs7gQ tGF3V498D0rhZO+n9PFMx4YOmH2zMjivmqOB2IdLQOXA6Q8CWp4nzCpObSp3pn3A nqLy71lasdyWTxmIRyylPtsTUEHmaMhVhvKTjUgcTE5Te1SP1bsBkRJdCSHX+Rzc 4Q6iWJSMOA+7znVC+lEL =pjGQ -----END PGP SIGNATURE----- --Bk/e7+bhc7oa9AZn--