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 [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to