From owner-svn-src-all@FreeBSD.ORG Mon Dec 1 21:32:23 2008 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 828111065707; Mon, 1 Dec 2008 21:32:23 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.terabit.net.ua (mail.terabit.net.ua [195.137.202.147]) by mx1.freebsd.org (Postfix) with ESMTP id 164AB8FC0C; Mon, 1 Dec 2008 21:32:23 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from skuns.zoral.com.ua ([91.193.166.194] helo=mail.zoral.com.ua) by mail.terabit.net.ua with esmtps (TLSv1:AES256-SHA:256) (Exim 4.63 (FreeBSD)) (envelope-from ) id 1L7GNR-000AQ3-9g; Mon, 01 Dec 2008 23:32:21 +0200 Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id mB1LWINh096678 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 1 Dec 2008 23:32:18 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3) with ESMTP id mB1LWHGG050969; Mon, 1 Dec 2008 23:32:17 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3/Submit) id mB1LWH70050967; Mon, 1 Dec 2008 23:32:17 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 1 Dec 2008 23:32:17 +0200 From: Kostik Belousov To: John Baldwin Message-ID: <20081201213217.GR3045@deviant.kiev.zoral.com.ua> References: <200811221311.mAMDBBU8018510@svn.freebsd.org> <200812011336.37636.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="v4cNTr+tRGSs1txX" Content-Disposition: inline In-Reply-To: <200812011336.37636.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.93.3, clamav-milter version 0.93.3 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua X-Virus-Scanned: mail.terabit.net.ua 1L7GNR-000AQ3-9g e6ea09d767801372827fdea8673836b4 X-Terabit: YES Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r185170 - head/sys/ufs/ufs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Dec 2008 21:32:23 -0000 --v4cNTr+tRGSs1txX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Dec 01, 2008 at 01:36:36PM -0500, John Baldwin wrote: > On Saturday 22 November 2008 08:11:11 am Konstantin Belousov wrote: > > Author: kib > > Date: Sat Nov 22 13:11:11 2008 > > New Revision: 185170 > > URL: http://svn.freebsd.org/changeset/base/185170 > >=20 > > Log: > > Busy ufs filesystem around block of code that does ".." lookup. Since > > mnt_lock is before lock of any vnode on the mp, it uses LK_NOWAIT. Si= nce > > MNTK_UNMOUNT may be transient, pdp lock is dropped when vfs_busy() > > failed, and operation is retried after some time. This way, ffs_vget() > > is not called on the mp that may be in the process of being destroyed= by > > unmount. > > =20 > > Check for the VI_DOOMED flag on pdp after its lock is reacquired, to > > better detect some situations where directory containing ".." > > entry is removed during the lookup. >=20 > I'm not really sure it matters if the parent directory goes away because = it=20 > will have deadfs vops so any subsequent operations will already fail, yes? Operations will fail. There is another race with parent directory being removed while pdp is unlocked. This can creep without tripping over deadfs operations for pdp. As Tor noted, the race may be considered as a security issue, allowing to escape the chroot. Check for reclamation cannot catch a move of pdp, this is why I specified the check as partial measure. >=20 > Also, do you really need to grab the VI_LOCK just to check VI_DOOMED? O= ther=20 > places in the kernel check that flag while holding the vnode lock w/o=20 > acquiring the interlock. Since you are just doing a single atomic read t= he=20 > interlock doesn't actually close any races anyway. I think it just adds= =20 > overhead. Yes, VI_DOOMED is set when both lock and interlock is held. I will remove interlock around the check. >=20 > > Reviewed by: tegge, attilio (previous version) > > Tested by: pho > > MFC after: 1 month > >=20 > > Modified: > > head/sys/ufs/ufs/ufs_lookup.c > >=20 > > Modified: head/sys/ufs/ufs/ufs_lookup.c > >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > > --- head/sys/ufs/ufs/ufs_lookup.c Sat Nov 22 12:36:15 2008 (r185169) > > +++ head/sys/ufs/ufs/ufs_lookup.c Sat Nov 22 13:11:11 2008 (r185170) > > @@ -157,6 +157,8 @@ ufs_lookup(ap) > > int nameiop =3D cnp->cn_nameiop; > > ino_t ino; > > int ltype; > > + int pdoomed; > > + struct mount *mp; > > =20 > > bp =3D NULL; > > slotoffset =3D -1; > > @@ -578,9 +580,32 @@ found: > > pdp =3D vdp; > > if (flags & ISDOTDOT) { > > ltype =3D VOP_ISLOCKED(pdp); > > + mp =3D pdp->v_mount; > > + for (;;) { > > + error =3D vfs_busy(mp, MBF_NOWAIT); > > + if (error =3D=3D 0) > > + break; > > + VOP_UNLOCK(pdp, 0); > > + pause("ufs_dd", 1); > > + vn_lock(pdp, ltype | LK_RETRY); > > + VI_LOCK(pdp); > > + pdoomed =3D pdp->v_iflag & VI_DOOMED; > > + VI_UNLOCK(pdp); > > + if (pdoomed) > > + return (ENOENT); > > + } > > VOP_UNLOCK(pdp, 0); /* race to get the inode */ > > - error =3D VFS_VGET(pdp->v_mount, ino, cnp->cn_lkflags, &tdp); > > + error =3D VFS_VGET(mp, ino, cnp->cn_lkflags, &tdp); > > + vfs_unbusy(mp); > > vn_lock(pdp, ltype | LK_RETRY); > > + VI_LOCK(pdp); > > + pdoomed =3D pdp->v_iflag & VI_DOOMED; > > + VI_UNLOCK(pdp); > > + if (pdoomed) { > > + if (error =3D=3D 0) > > + vput(tdp); > > + error =3D ENOENT; > > + } > > if (error) > > return (error); > > *vpp =3D tdp; > >=20 >=20 >=20 >=20 > --=20 > John Baldwin --v4cNTr+tRGSs1txX Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkk0V+EACgkQC3+MBN1Mb4gTLgCgp1rQjMHM3kgoMCtr9z4rRZj3 WXcAmwTnDuAxe5QYEjvnKQArxRDdxOf0 =EIHL -----END PGP SIGNATURE----- --v4cNTr+tRGSs1txX--