Re: kern/41051: do_sys_rename mp resource race

[Antti Kantee <pooka%iki.fi@localhost>]

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

From: Antti Kantee <pooka%iki.fi@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/41051: do_sys_rename mp resource race
Date: Sun, 22 Mar 2009 06:20:44 +0200

 On Sat Mar 21 2009 at 14:35:01 +0000, Andrew Doran wrote:
 > The following reply was made to PR kern/41051; it has been noted by GNATS.
 > 
 > From: Andrew Doran <ad%netbsd.org@localhost>
 > To: gnats-bugs%NetBSD.org@localhost
 > Cc: 
 > Subject: Re: kern/41051: do_sys_rename mp resource race
 > Date: Sat, 21 Mar 2009 14:34:36 +0000
 > 
 >  On Sat, Mar 21, 2009 at 08:40:03AM +0000, Antti Kantee wrote:
 >  
 >  >  >  > take reference to mp around rename
 >  >  >  
 >  >  >  Disagree. I think VOPs should not, in general, be dropping refs to 
 > vnodes
 >  >  >  that the caller provides.
 >  >  
 >  >  do_sys_rename() is not a VOP, neither is vfs_destroy().  What do you
 >  >  mean by a VOP here?
 >  
 >  Well in general the model is that references propagate. So if you hold a
 >  reference at some point, say a a file descriptor, everything down the chain
 >  from that persists, be it a file, vnode, mount, socket - whatever. I'm
 >  guessing you probably already grok that.
 >  
 >  VOP_RENAME() is a pariah here because it tosses the caller's dvp references.
 >  On the way in both mount structures involved are valid but after it returns,
 >  either can be gone. So the operation is not logically complete from the
 >  driver's perspective (do_sys_rename) but the objects involved are already
 >  gone.
 
 Could you explain what you want to do about the problem.  I have a
 hard time interpreting the above in any other way except that you
 are disagreeing with the suggested fix because you think file system
 locking should be completely reworked so that this problem with rename
 can be fixed.  I do agree that improvements in that area are necessary,
 but I think that is way out of scope here, especially considering the
 release branch.
 
 (also, do_sys_rename handles only one mountpoint, the rest is up to the
 file system, but that's a minor detail)