Re: kern/40163: mv(1) cause WAPBL panic

[Greg Oster <oster%cs.usask.ca@localhost>]

The following reply was made to PR kern/40163; it has been noted by GNATS.

From: Greg Oster <oster%cs.usask.ca@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/40163: mv(1) cause WAPBL panic 
Date: Fri, 12 Dec 2008 22:47:09 -0600

 David Holland writes:
 > The following reply was made to PR kern/40163; it has been noted by GNATS.
 > 
 > From: David Holland <dholland-bugs%netbsd.org@localhost>
 > To: gnats-bugs%netbsd.org@localhost
 > Cc: kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
 >      netbsd-bugs%netbsd.org@localhost, 
 > lloyd%must-have-coffee.gen.nz@localhost
 > Subject: Re: kern/40163: mv(1) cause WAPBL panic
 > Date: Sat, 13 Dec 2008 03:52:09 +0000
 > 
 >  On Sat, Dec 13, 2008 at 02:55:02AM +0000, Greg Oster wrote:
 >   >>> Typing "mv foo/foo ." causes a panic if the filesystem in question has 
 > log
 >   >> ging enabled.
 >   >>  
 >   >>  I am able to reproduce this on 5.0 also, note that foo/foo must be a 
 >   >>  directory and not a regular file.
 >   >  
 >   >  Ah! That is the key..
 >   > [...]
 >   >  345             if (newparent) {
 >   >  346                     vn_lock(fdvp, LK_EXCLUSIVE | LK_RETRY);
 >  
 >  Untested (I don't have any wapbl volumes on hand) patch:
 >  
 >  Index: ufs_wapbl.c
 >  ===================================================================
 >  RCS file: /cvsroot/src/sys/ufs/ufs/ufs_wapbl.c,v
 >  retrieving revision 1.3
 >  diff -u -p -r1.3 ufs_wapbl.c
 >  --- ufs_wapbl.c     8 Dec 2008 11:37:37 -0000       1.3
 >  +++ ufs_wapbl.c     13 Dec 2008 03:49:29 -0000
 >  @@ -343,6 +343,11 @@ wapbl_ufs_rename(void *v)
 >      fcnp->cn_flags &= ~(MODMASK | SAVESTART);
 >      fcnp->cn_flags |= LOCKPARENT | LOCKLEAF;
 >      if (newparent) {
 >  +           /* Check for the rename("foo/foo", "foo") case. */
 >  +           if (fdvp == tvp) {
 >  +                   error = doingdirectory ? ENOTEMPTY : ENOTDIR;
 >  +                   goto out;
 >  +           }
 >              vn_lock(fdvp, LK_EXCLUSIVE | LK_RETRY);
 >              if ((error = relookup(fdvp, &fvp, fcnp))) {
 >                      vput(fdvp);
 
 I now get:
 
 cow# mv foo/foo .
 mv: rename foo/foo to ./foo: Directory not empty
 
 as expected... 
  
 >  Also, someone should squint at this to make sure "out" is the right
 >  error exit path here.
 
 The light squinting I did made me think it's ok, but it should 
 probably be verified by better squinters...
 
 Later...
 
 Greg Oster