From owner-freebsd-audit@FreeBSD.ORG Wed Jan 12 13:53:39 2005 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id F114E16A4CE; Wed, 12 Jan 2005 13:53:38 +0000 (GMT) Received: from heechee.tobez.org (heechee.tobez.org [217.157.39.226]) by mx1.FreeBSD.org (Postfix) with ESMTP id B7C6343D46; Wed, 12 Jan 2005 13:53:37 +0000 (GMT) (envelope-from tobez@tobez.org) Received: by heechee.tobez.org (Postfix, from userid 1001) id 81BD7125465; Wed, 12 Jan 2005 14:53:36 +0100 (CET) Date: Wed, 12 Jan 2005 14:53:36 +0100 From: Anton Berezin To: freebsd-audit@freebsd.org Message-ID: <20050112135336.GC68344@heechee.tobez.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.2.1i cc: DougB@FreeBSD.org Subject: [PATCH] review requested, add a feature to mergemaster X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jan 2005 13:53:39 -0000 Following the idea by Sune Stjerneby, suggestions from Andrzej Tobola and Phil Regnauld, and a bit of discussion on -current, I would like to request a review of a new mergemaster feature aimed to simplify manual work in some cases. A new option is added: -R cvsroot For files that contain a $FreeBSD$ string, but which does not match the one in the corresponding file in the temporary root, attempt to extract the same version from the FreeBSD CVS repository specified by cvsroot. The mergemaster utility will then compare the CVS version with the installed version using diff(1). If they are identical, mergemaster will assume that the file in the destination directory was not modified and therefore can be safely overwritten by the ver- sion in the temporary root directory. The patch is against fresh HEAD. Index: mergemaster.8 =================================================================== RCS file: /home/ncvs/src/usr.sbin/mergemaster/mergemaster.8,v retrieving revision 1.29 diff -u -r1.29 mergemaster.8 --- mergemaster.8 2 Jul 2004 23:12:48 -0000 1.29 +++ mergemaster.8 12 Jan 2005 13:41:25 -0000 @@ -39,6 +39,7 @@ .Op Fl u Ar N .Op Fl w Ar N .Op Fl D Ar /path +.Op Fl R Ar cvsroot .Sh DESCRIPTION The .Nm @@ -222,6 +223,23 @@ .Pa /path/to/temp/root instead of the default .Pa /var/tmp/temproot . +.It Fl R Ar cvsroot +For files that contain a $FreeBSD$ string, +but which does not match the one in the corresponding file +in the temporary root, +attempt to extract the same version from the +.Fx +CVS repository specified by +.Ar cvsroot. +The +.Nm +utility will then compare the CVS version with the installed version using +.Xr diff 1 . +If they are identical, +.Nm +will assume that the file in the destination directory +was not modified and therefore can be safely +overwritten by the version in the temporary root directory. .It Fl d Add the date and time to the name of the temporary root directory. Index: mergemaster.sh =================================================================== RCS file: /home/ncvs/src/usr.sbin/mergemaster/mergemaster.sh,v retrieving revision 1.51 diff -u -r1.51 mergemaster.sh --- mergemaster.sh 7 Mar 2004 10:10:19 -0000 1.51 +++ mergemaster.sh 12 Jan 2005 13:52:11 -0000 @@ -30,6 +30,7 @@ echo ' -P Preserve files that are overwritten' echo " -m /path/directory Specify location of source to do the make in" echo " -t /path/directory Specify temp root directory" + echo " -R cvsroot Specify cvs repository and instruct to use it" echo " -d Add date and time to directory name (e.g., /var/tmp/temproot.`date +%m%d.%H.%M`)" echo " -u N Specify a numeric umask" echo " -w N Specify a screen width in columns to sdiff" @@ -238,7 +239,7 @@ # Check the command line options # -while getopts ":ascrvhipCPm:t:du:w:D:" COMMAND_LINE_ARGUMENT ; do +while getopts ":ascrvhipCPm:t:R:du:w:D:" COMMAND_LINE_ARGUMENT ; do case "${COMMAND_LINE_ARGUMENT}" in s) STRICT=yes @@ -284,6 +285,9 @@ t) TEMPROOT=${OPTARG} ;; + R) + CVSROOT=${OPTARG} + ;; d) TEMPROOT=${TEMPROOT}.`date +%m%d.%H.%M` ;; @@ -896,21 +900,59 @@ echo " *** Temp ${COMPFILE} and installed are the same, deleting" rm "${COMPFILE}" else - # Ok, the files are different, so show the user where they differ. - # Use user's choice of diff methods; and user's pager if they have one. - # Use more if not. - # Use unified diffs by default. Context diffs give me a headache. :) - # - case "${AUTO_RUN}" in + DO_DIFF_LOOP=yes + + case "${CVSROOT}" in '') - # prompt user to install/delete/merge changes - diff_loop ;; *) - # If this is an auto run, make it official - echo " *** ${COMPFILE} will remain for your consideration" - ;; - esac # Auto run test + CVSFILE=`grep "[$]${CVS_ID_TAG}:" ${DESTDIR}${COMPFILE#.} 2>/dev/null | + sed -e "s#.*[$]${CVS_ID_TAG}: \(src/.*\),v .*#\1#" 2>/dev/null` + CVSREV=`grep "[$]${CVS_ID_TAG}:" ${DESTDIR}${COMPFILE#.} 2>/dev/null | + sed -e "s#.*[$]${CVS_ID_TAG}: src/.*,v \([0-9.]*\) .*#\1#" 2>/dev/null` + if [ -n "${CVSFILE}" -a -n "${CVSREV}" ]; then + CVSFILE_N=`echo "${CVSFILE}"|wc -l` + CVSREV_N=`echo "${CVSREV}"|wc -l` + if [ ${CVSFILE_N} -eq 1 -a ${CVSREV_N} -eq 1 ]; then + CVSTEMP=`mktemp -t mmcvs` + cvs -d ${CVSROOT} co -r${CVSREV} -p ${CVSFILE} >${CVSTEMP} 2>/dev/null + if diff -q ${DIFF_OPTIONS} "${DESTDIR}${COMPFILE#.}" "${CVSTEMP}" > \ + /dev/null 2>&1; then + rm -f ${CVSTEMP} + echo '' + echo " *** The installed version of ${COMPFILE} is the same as" + echo " its CVS revision ${CVSREV}, overwriting it" + echo '' + if mm_install "${COMPFILE}"; then + echo " *** ${COMPFILE} installed successfully" + else + echo " *** Problem installing ${COMPFILE}, it will remain to merge by hand" + fi + unset DO_DIFF_LOOP + fi # same file in CVS and in DESTDIR, can overwrite it + rm -f ${CVSTEMP} + fi # CVSFILE and CVSREV can be used + fi # CVSFILE and CVSREV non-empty + ;; + esac # cvs root test + + if [ -n "${DO_DIFF_LOOP}" ]; then + # Ok, the files are different, so show the user where they differ. + # Use user's choice of diff methods; and user's pager if they have one. + # Use more if not. + # Use unified diffs by default. Context diffs give me a headache. :) + # + case "${AUTO_RUN}" in + '') + # prompt user to install/delete/merge changes + diff_loop + ;; + *) + # If this is an auto run, make it official + echo " *** ${COMPFILE} will remain for your consideration" + ;; + esac # Auto run test + fi # Yes, do a diff loop fi # Yes, the files are different fi # Yes, the file still remains to be checked done # This is for the do way up there at the beginning of the comparison Cheers, \Anton. -- The moronity of the universe is a monotonically increasing function. -- Jarkko Hietaniemi From owner-freebsd-audit@FreeBSD.ORG Wed Jan 12 20:34:54 2005 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D6FCE16A4CE; Wed, 12 Jan 2005 20:34:54 +0000 (GMT) Received: from smtp1.server.rpi.edu (smtp1.server.rpi.edu [128.113.2.1]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6581943D31; Wed, 12 Jan 2005 20:34:54 +0000 (GMT) (envelope-from drosih@rpi.edu) Received: from [128.113.24.47] (gilead.netel.rpi.edu [128.113.24.47]) by smtp1.server.rpi.edu (8.13.0/8.13.0) with ESMTP id j0CKYpcv019259; Wed, 12 Jan 2005 15:34:53 -0500 Mime-Version: 1.0 Message-Id: In-Reply-To: <20050112135336.GC68344@heechee.tobez.org> References: <20050112135336.GC68344@heechee.tobez.org> Date: Wed, 12 Jan 2005 15:34:50 -0500 To: Anton Berezin , freebsd-audit@freebsd.org From: Garance A Drosihn Content-Type: text/plain; charset="us-ascii" ; format="flowed" X-CanItPRO-Stream: default X-RPI-SA-Score: undef - spam-scanning disabled X-Scanned-By: CanIt (www . canit . ca) cc: DougB@freebsd.org Subject: Re: [PATCH] review requested, add a feature to mergemaster X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jan 2005 20:34:55 -0000 At 2:53 PM +0100 1/12/05, Anton Berezin wrote: >Following the idea by Sune Stjerneby, suggestions from Andrzej >Tobola and Phil Regnauld, and a bit of discussion on -current, >I would like to request a review of a new mergemaster feature >aimed to simplify manual work in some cases. > >A new option is added: > > -R cvsroot For files that contain a $FreeBSD$ string, Hmm. I'm not sure how useful this will be (not that I object to it in anyway, I'm just not sure...). But let me note: >=================================================================== >RCS file: /home/ncvs/src/usr.sbin/mergemaster/mergemaster.8,v >retrieving revision 1.29 >diff -u -r1.29 mergemaster.8 >+.It Fl R Ar cvsroot >+For files that contain a $FreeBSD$ string, >+but which does not match the one in the corresponding file >+in the temporary root, >+attempt to extract the same version from the >+.Fx >+CVS repository specified by >+.Ar cvsroot. Be careful when trying to add the literal string of $FreeBSD$ to any documentation file. This will look fine when you are testing it, but when you *commit* it, CVS will expand that string into the FreeBSD-version information for that file. I forget what you have to do to get the right results, but you don't want to have the literal '$FreeBSD$' string in the file. -- Garance Alistair Drosehn = gad@gilead.netel.rpi.edu Senior Systems Programmer or gad@freebsd.org Rensselaer Polytechnic Institute or drosih@rpi.edu From owner-freebsd-audit@FreeBSD.ORG Wed Jan 12 21:17:40 2005 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E4DC416A4CE; Wed, 12 Jan 2005 21:17:40 +0000 (GMT) Received: from heechee.tobez.org (heechee.tobez.org [217.157.39.226]) by mx1.FreeBSD.org (Postfix) with ESMTP id E4A4B43D1D; Wed, 12 Jan 2005 21:17:39 +0000 (GMT) (envelope-from tobez@tobez.org) Received: by heechee.tobez.org (Postfix, from userid 1001) id 085A8125467; Wed, 12 Jan 2005 22:17:38 +0100 (CET) Date: Wed, 12 Jan 2005 22:17:38 +0100 From: Anton Berezin To: Garance A Drosihn Message-ID: <20050112211737.GB15577@heechee.tobez.org> References: <20050112135336.GC68344@heechee.tobez.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.1i cc: DougB@freebsd.org cc: freebsd-audit@freebsd.org Subject: Re: [PATCH] review requested, add a feature to mergemaster X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jan 2005 21:17:41 -0000 On Wed, Jan 12, 2005 at 03:34:50PM -0500, Garance A Drosihn wrote: > At 2:53 PM +0100 1/12/05, Anton Berezin wrote: > >Following the idea by Sune Stjerneby, suggestions from Andrzej > >Tobola and Phil Regnauld, and a bit of discussion on -current, > >I would like to request a review of a new mergemaster feature > >aimed to simplify manual work in some cases. > > > >A new option is added: > > > > -R cvsroot For files that contain a $FreeBSD$ string, > > Hmm. I'm not sure how useful this will be (not that I object > to it in anyway, I'm just not sure...). Depending on the amount of tinkering done to /etc files, which is an undertemined function of machine function, administrator preferences and local policy, it should in theory substantially reduce pointless qi, which are not only time-consuming, but also tend to make an admin less attentive, possibly resulting in qi *OOPS* (I've done that in the past). A single, probably not very useful datapoint: install 5.3-RELEASE, cvsup to RELENG_5, {build,install}{world,kernel}, then do mergemaster. This currently leads to about 20 cases of "pointless diffs", all of which go away when using this option. If a concern is that only a minority of machines have /home/ncvs installed, then the answer would be that "cvsroot" is a generic string, which can be anything cvs -d accepts, meaning that various :pserver: and :ext:-based solutions shall work. > But let me note: > >+For files that contain a $FreeBSD$ string, > Be careful when trying to add the literal string of $FreeBSD$ to any > documentation file. This will look fine when you are testing it, but > when you *commit* it, CVS will expand that string into the > FreeBSD-version information for that file. Thanks, that shall be changed to just "$FreeBSD", as it already appears elsewhere in the manual. \Anton. -- The moronity of the universe is a monotonically increasing function. -- Jarkko Hietaniemi From owner-freebsd-audit@FreeBSD.ORG Wed Jan 12 21:35:48 2005 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C570E16A4CE; Wed, 12 Jan 2005 21:35:48 +0000 (GMT) Received: from smtp1.server.rpi.edu (smtp1.server.rpi.edu [128.113.2.1]) by mx1.FreeBSD.org (Postfix) with ESMTP id 686D643D46; Wed, 12 Jan 2005 21:35:48 +0000 (GMT) (envelope-from drosih@rpi.edu) Received: from [128.113.24.47] (gilead.netel.rpi.edu [128.113.24.47]) by smtp1.server.rpi.edu (8.13.0/8.13.0) with ESMTP id j0CLZkBA005620; Wed, 12 Jan 2005 16:35:47 -0500 Mime-Version: 1.0 Message-Id: In-Reply-To: <20050112211737.GB15577@heechee.tobez.org> References: <20050112135336.GC68344@heechee.tobez.org> <20050112211737.GB15577@heechee.tobez.org> Date: Wed, 12 Jan 2005 16:35:45 -0500 To: Anton Berezin From: Garance A Drosihn Content-Type: text/plain; charset="us-ascii" ; format="flowed" X-CanItPRO-Stream: default X-RPI-SA-Score: undef - spam-scanning disabled X-Scanned-By: CanIt (www . canit . ca) cc: DougB@freebsd.org cc: freebsd-audit@freebsd.org Subject: Re: [PATCH] review requested, add a feature to mergemaster X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jan 2005 21:35:48 -0000 At 10:17 PM +0100 1/12/05, Anton Berezin wrote: >On Wed, Jan 12, 2005, Garance A Drosihn wrote: > > At 2:53 PM +0100 1/12/05, Anton Berezin wrote: > > > >> >A new option is added: >> > >> > -R cvsroot For files that contain a $FreeBSD$ string, > > >> Hmm. I'm not sure how useful this will be (not that I object > > to it in anyway, I'm just not sure...). >A single, probably not very useful datapoint: install 5.3-RELEASE, >cvsup to RELENG_5, {build,install}{world,kernel}, then do mergemaster. >This currently leads to about 20 cases of "pointless diffs", all of >which go away when using this option. I understand the desired goal. I'm just not sure that this option will be generally useful. >If a concern is that only a minority of machines have /home/ncvs >installed, then the answer would be that "cvsroot" is a generic >string, which can be anything cvs -d accepts, meaning that >various :pserver: and :ext:-based solutions shall work. If a user is following the rules for upgrading, then the really important mergemaster run is done in single-user mode. You do not have networking, you do not have NFS, you do not even have the any value set for `hostname`. Thus, if that user does not have the cvs repository on their local hard disk, then this option will require several additional steps (and those steps may not be convenient, particularly for someone who gets their network address via DHCP). Again, let me point out that I am not objecting to this idea. I am only saying that it may not be as useful as it first seems to be. > > But let me note: > >> >+For files that contain a $FreeBSD$ string, > >> Be careful when trying to add the literal string of $FreeBSD$ to any >> documentation file. This will look fine when you are testing it, but >> when you *commit* it, CVS will expand that string into the >> FreeBSD-version information for that file. > >Thanks, that shall be changed to just "$FreeBSD", as it already >appears elsewhere in the manual. Sounds good. That was the main purpose of my message. -- Garance Alistair Drosehn = gad@gilead.netel.rpi.edu Senior Systems Programmer or gad@freebsd.org Rensselaer Polytechnic Institute or drosih@rpi.edu From owner-freebsd-audit@FreeBSD.ORG Wed Jan 12 21:41:45 2005 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 28D2C16A4CE; Wed, 12 Jan 2005 21:41:45 +0000 (GMT) Received: from odin.ac.hmc.edu (Odin.AC.HMC.Edu [134.173.32.75]) by mx1.FreeBSD.org (Postfix) with ESMTP id EBD8843D31; Wed, 12 Jan 2005 21:41:44 +0000 (GMT) (envelope-from brdavis@odin.ac.hmc.edu) Received: from odin.ac.hmc.edu (localhost.localdomain [127.0.0.1]) by odin.ac.hmc.edu (8.13.0/8.13.0) with ESMTP id j0CLifwb023692; Wed, 12 Jan 2005 13:44:41 -0800 Received: (from brdavis@localhost) by odin.ac.hmc.edu (8.13.0/8.13.0/Submit) id j0CLifPc023691; Wed, 12 Jan 2005 13:44:41 -0800 Date: Wed, 12 Jan 2005 13:44:41 -0800 From: Brooks Davis To: Garance A Drosihn Message-ID: <20050112214441.GB21038@odin.ac.hmc.edu> References: <20050112135336.GC68344@heechee.tobez.org> <20050112211737.GB15577@heechee.tobez.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="CUfgB8w4ZwR/yMy5" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.1i X-Virus-Scanned: by amavisd-new X-Spam-Status: No, hits=0.0 required=8.0 tests=none autolearn=no version=2.63 X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on odin.ac.hmc.edu cc: DougB@freebsd.org cc: Anton Berezin cc: freebsd-audit@freebsd.org Subject: Re: [PATCH] review requested, add a feature to mergemaster X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jan 2005 21:41:45 -0000 --CUfgB8w4ZwR/yMy5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jan 12, 2005 at 04:35:45PM -0500, Garance A Drosihn wrote: > At 10:17 PM +0100 1/12/05, Anton Berezin wrote: > >On Wed, Jan 12, 2005, Garance A Drosihn wrote: > > > At 2:53 PM +0100 1/12/05, Anton Berezin wrote: > > > > > >> >A new option is added: > >> > > >> > -R cvsroot For files that contain a $FreeBSD$ string, > > > > >> Hmm. I'm not sure how useful this will be (not that I object > > > to it in anyway, I'm just not sure...). >=20 >=20 > >A single, probably not very useful datapoint: install 5.3-RELEASE, > >cvsup to RELENG_5, {build,install}{world,kernel}, then do mergemaster. > >This currently leads to about 20 cases of "pointless diffs", all of > >which go away when using this option. >=20 > I understand the desired goal. I'm just not sure that this > option will be generally useful. >=20 > >If a concern is that only a minority of machines have /home/ncvs > >installed, then the answer would be that "cvsroot" is a generic > >string, which can be anything cvs -d accepts, meaning that > >various :pserver: and :ext:-based solutions shall work. >=20 > If a user is following the rules for upgrading, then the really > important mergemaster run is done in single-user mode. You do > not have networking, you do not have NFS, you do not even have the > any value set for `hostname`. Thus, if that user does not have > the cvs repository on their local hard disk, then this option will > require several additional steps (and those steps may not be > convenient, particularly for someone who gets their network > address via DHCP). Given the existance of /etc/netstart, I don't think this is a big deal. In any case, there is rairly a need to actually do mergemaster in single user mode except on production servers. -- Brooks --=20 Any statement of the form "X is the one, true Y" is FALSE. PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4 --CUfgB8w4ZwR/yMy5 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQFB5ZpIXY6L6fI4GtQRAtbkAKDNlWA0oiQ42V0ObfBxd1+sXVQeTwCfVOyI 8H69aJ9GUV6BTpcR/d2o86M= =YMQr -----END PGP SIGNATURE----- --CUfgB8w4ZwR/yMy5-- From owner-freebsd-audit@FreeBSD.ORG Wed Jan 12 22:36:54 2005 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1652016A4CE; Wed, 12 Jan 2005 22:36:54 +0000 (GMT) Received: from smtp3.server.rpi.edu (smtp3.server.rpi.edu [128.113.2.3]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9DD6443D41; Wed, 12 Jan 2005 22:36:51 +0000 (GMT) (envelope-from drosih@rpi.edu) Received: from [128.113.24.47] (gilead.netel.rpi.edu [128.113.24.47]) by smtp3.server.rpi.edu (8.13.0/8.13.0) with ESMTP id j0CMaoMU000598; Wed, 12 Jan 2005 17:36:50 -0500 Mime-Version: 1.0 Message-Id: In-Reply-To: <20050112214441.GB21038@odin.ac.hmc.edu> References: <20050112135336.GC68344@heechee.tobez.org> <20050112211737.GB15577@heechee.tobez.org> <20050112214441.GB21038@odin.ac.hmc.edu> Date: Wed, 12 Jan 2005 17:36:49 -0500 To: Brooks Davis From: Garance A Drosihn Content-Type: text/plain; charset="us-ascii" ; format="flowed" X-CanItPRO-Stream: default X-RPI-SA-Score: undef - spam-scanning disabled X-Scanned-By: CanIt (www . canit . ca) cc: DougB@freebsd.org cc: Anton Berezin cc: freebsd-audit@freebsd.org Subject: Re: [PATCH] review requested, add a feature to mergemaster X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jan 2005 22:36:54 -0000 At 1:44 PM -0800 1/12/05, Brooks Davis wrote: > > >> If a user is following the rules for upgrading, then the really >> important mergemaster run is done in single-user mode. You do > > not have networking, you do not have NFS, you do not even have > > the any value set for `hostname`. [...] > >Given the existance of /etc/netstart, I don't think this is a >big deal. Ah. Yes, that does help a lot. -- Garance Alistair Drosehn = gad@gilead.netel.rpi.edu Senior Systems Programmer or gad@freebsd.org Rensselaer Polytechnic Institute or drosih@rpi.edu From owner-freebsd-audit@FreeBSD.ORG Thu Jan 13 22:15:49 2005 Return-Path: Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 31BA216A4CE; Thu, 13 Jan 2005 22:15:49 +0000 (GMT) Received: from fasterix.frmug.org (fasterix.frmug.org [82.226.244.120]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7042843D39; Thu, 13 Jan 2005 22:15:48 +0000 (GMT) (envelope-from pb@fasterix.frmug.org) Received: from fasterix.frmug.org (localhost [127.0.0.1]) by fasterix.frmug.org (8.13.1/8.13.1) with ESMTP id j0DMFkqZ003075 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 13 Jan 2005 23:15:46 +0100 (CET) (envelope-from pb@fasterix.frmug.org) Received: (from pb@localhost) by fasterix.frmug.org (8.13.1/8.13.1/Submit) id j0DMFkYf003074; Thu, 13 Jan 2005 23:15:46 +0100 (CET) (envelope-from pb) Date: Thu, 13 Jan 2005 23:15:46 +0100 From: Pierre Beyssac To: freebsd-audit@freebsd.org Message-ID: <20050113221546.GA2988@fasterix.frmug.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="pWyiEgJYm5f9v55/" Content-Disposition: inline User-Agent: Mutt/1.4.2.1i X-message-flag: Warning! Use of Microsoft Outlook is dangerous and makes your system susceptible to worms and viruses cc: wpaul@freebsd.org Subject: mutex patch for if_ndis X-BeenThere: freebsd-audit@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: FreeBSD Security Audit List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Jan 2005 22:15:49 -0000 --pWyiEgJYm5f9v55/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hello, The following patch for -current for if_ndis fixes a panic detected by the INVARIANTS code, at the msleep in kern_ndis.c:1562. > Assertion lock == sq->sq_lock failed at ../../../kern/subr_sleepqueue.c:286 > > sleepq_add(c1e4c560,c1f7ad68,c1f2bcac,c079edc0,0) at sleep_add+0x13d > msleep(c1f7ad68,c1f2bcac,259,c079edc0,1f4) at msleep+0x228 > ndis_get_info(c1f7a000,10114,e4a76b30,e4a76b34,c0633b48) at ndis_get_info+0x114 > ndis_ifmedia_sts(c1f7a000,e4a76b30,0,c049f6bc,c0632d20) at ndis_ifmedia_sts+0x45 It seems that the nmb_wkupdpctimer sleep queue can be slept on simultaneously by more than one thread, which causes sleepq_add() to fail an assertion (the reason of the panic is that the process going to sleep is not the process that acquired the lock). The patch simply replaces the per-process mutex with a dedicated mutex. It fixes the panic and I have tested it for several months now but as I'm not a mutex expert, I'd like to confirm that it's a suitable fix and that I've not left some obvious locking loophole before I commit it. -- Pierre Beyssac pb@fasterix.frmug.org pb@fasterix.freenix.org Free domains: http://www.eu.org/ or mail dns-manager@EU.org --pWyiEgJYm5f9v55/ Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=patch-ndis --- kern_ndis.c.orig Thu Sep 2 00:50:33 2004 +++ kern_ndis.c Sat Sep 25 23:23:41 2004 @@ -110,6 +110,7 @@ static uma_zone_t ndis_packet_zone, ndis_buffer_zone; struct mtx ndis_thr_mtx; +struct mtx ndis_req_mtx; static STAILQ_HEAD(ndisqhead, ndis_req) ndis_ttodo; struct ndisqhead ndis_itodo; struct ndisqhead ndis_free; @@ -259,6 +260,8 @@ mtx_init(&ndis_thr_mtx, "NDIS thread lock", MTX_NDIS_LOCK, MTX_DEF); + mtx_init(&ndis_req_mtx, "NDIS request lock", + MTX_NDIS_LOCK, MTX_DEF); STAILQ_INIT(&ndis_ttodo); STAILQ_INIT(&ndis_itodo); @@ -317,6 +320,7 @@ free(r, M_DEVBUF); } + mtx_destroy(&ndis_req_mtx); mtx_destroy(&ndis_thr_mtx); return; @@ -509,8 +513,8 @@ { int error; - PROC_LOCK(p); - error = msleep(&p->p_siglist, &p->p_mtx, + mtx_lock(&ndis_req_mtx); + error = msleep(&p->p_siglist, &ndis_req_mtx, curthread->td_priority|PDROP, "ndissp", timo); return(error); } @@ -1138,9 +1142,9 @@ ntoskrnl_lower_irql(irql); if (rval == NDIS_STATUS_PENDING) { - PROC_LOCK(curthread->td_proc); + mtx_lock(&ndis_req_mtx); error = msleep(&sc->ndis_block.nmb_wkupdpctimer, - &curthread->td_proc->p_mtx, + &ndis_req_mtx, curthread->td_priority|PDROP, "ndisset", 5 * hz); rval = sc->ndis_block.nmb_setstat; @@ -1322,8 +1326,8 @@ ntoskrnl_lower_irql(irql); if (rval == NDIS_STATUS_PENDING) { - PROC_LOCK(curthread->td_proc); - msleep(sc, &curthread->td_proc->p_mtx, + mtx_lock(&ndis_req_mtx); + msleep(sc, &ndis_req_mtx, curthread->td_priority|PDROP, "ndisrst", 0); } @@ -1558,9 +1562,9 @@ /* Wait for requests that block. */ if (rval == NDIS_STATUS_PENDING) { - PROC_LOCK(curthread->td_proc); + mtx_lock(&ndis_req_mtx); error = msleep(&sc->ndis_block.nmb_wkupdpctimer, - &curthread->td_proc->p_mtx, + &ndis_req_mtx, curthread->td_priority|PDROP, "ndisget", 5 * hz); rval = sc->ndis_block.nmb_getstat; --pWyiEgJYm5f9v55/--