From owner-freebsd-current Tue Jul 9 8:59: 5 2002 Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2548137B400 for ; Tue, 9 Jul 2002 08:59:01 -0700 (PDT) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id CFE0743E58 for ; Tue, 9 Jul 2002 08:58:59 -0700 (PDT) (envelope-from dl-freebsd@catspoiler.org) Received: from mousie.catspoiler.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.5/8.12.5) with ESMTP id g69Fwcwr004219; Tue, 9 Jul 2002 08:58:43 -0700 (PDT) (envelope-from dl-freebsd@catspoiler.org) Message-Id: <200207091558.g69Fwcwr004219@gw.catspoiler.org> Date: Tue, 9 Jul 2002 08:58:38 -0700 (PDT) From: Don Lewis Subject: code ordering in coredump() (was: Re: cvs commit: src/sys/tools vnode_if.awk) To: jroberson@chesapeake.net, current@FreeBSD.ORG Cc: jroberson@chesapeake.net, current@FreeBSD.ORG In-Reply-To: <200207081423.g68ENiwr000522@gw.catspoiler.org> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG I was studying the following DEBUG_VFS_LOCKS panic and noticed something bothersome about the ordering of the code in coredump(). It looked to me like it made more sense to verify that the file was something that was valid to dump to before doing the vn_start_write() stuff. Rearranging the code also allows VOP_GETATTR() to be called before dropping the initial lock. > VOP_GETATTR: 0xc7445100 is not locked but should be > Debugger("Lock violation. > ") > Stopped at Debugger+0x45: xchgl %ebx,in_Debugger.0 > db> tr > Debugger(c041b364) at Debugger+0x45 > coredump(c6a2c180) at coredump+0x470 > sigexit(c6a2c180,b,0,e54f8000,c6a2c180) at sigexit+0xa4 > postsig(b) at postsig+0xe9 > ast(e5514d48) at ast+0x27e > doreti_ast() at doreti_ast+0x1a Index: kern_sig.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_sig.c,v retrieving revision 1.174 diff -u -r1.174 kern_sig.c --- kern_sig.c 3 Jul 2002 09:15:20 -0000 1.174 +++ kern_sig.c 9 Jul 2002 15:17:31 -0000 @@ -1967,8 +1967,6 @@ * then it passes on a vnode and a size limit to the process-specific * coredump routine if there is one; if there _is not_ one, it returns * ENOSYS; otherwise it returns the error from the process-specific routine. - * - * XXX: VOP_GETATTR() here requires holding the vnode lock. */ static int @@ -2021,6 +2019,14 @@ NDFREE(&nd, NDF_ONLY_PNBUF); vp = nd.ni_vp; + /* Don't dump to non-regular files or files with links. */ + if (vp->v_type != VREG || + VOP_GETATTR(vp, &vattr, cred, td) || vattr.va_nlink != 1) { + VOP_UNLOCK(vp, 0, td); + error = EFAULT; + goto out2; + } + VOP_UNLOCK(vp, 0, td); lf.l_whence = SEEK_SET; lf.l_start = 0; @@ -2040,12 +2046,6 @@ goto restart; } - /* Don't dump to non-regular files or files with links. */ - if (vp->v_type != VREG || - VOP_GETATTR(vp, &vattr, cred, td) || vattr.va_nlink != 1) { - error = EFAULT; - goto out1; - } VATTR_NULL(&vattr); vattr.va_size = 0; vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td); @@ -2060,7 +2060,6 @@ p->p_sysent->sv_coredump(td, vp, limit) : ENOSYS; -out1: lf.l_type = F_UNLCK; VOP_ADVLOCK(vp, (caddr_t)p, F_UNLCK, &lf, F_FLOCK); vn_finished_write(mp); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message