Re: ufs_rename() fix
This is a discussion on Re: ufs_rename() fix within the mailing.openbsd.tech forums, part of the OpenBSD category; --> people running with this, please speak up. people not running with this, please do so. the huge amount of ...
| |||||||
| FAQ | Members List | Calendar | Search | Today's Posts | Mark Forums Read |
02-18-2008, 08:46 AM
| ||||
| ||||
 Re: ufs_rename() fix people running with this, please speak up. people not running with this, please do so. the huge amount of 2 reports i got was kind of, well, disappointing. -p. On Tue, Apr 12, 2005 at 12:02:59PM -0300, Pedro Martelletto wrote: > hi, > > this present diff fixes two races currently in our ufs_rename() code, > both due to failed node lookups, in which cases we should not carry on > with the rename process, the problem being more easily triggered in the > presence of many similar renames happening simultaneously, a behaviour > busy postfix servers seem to expose quite commonly, as can be seen by > the PRs 4040 and 4126, whose submitters have been already running with > this fix for a while, both having reported success, and PR 4169 as well. > > in addition, this diff also addresses a reference that was being lost > in ufs_checkpath() and optimizes a if/else's logical structure. the fix > for the two relevant problems were taken from netbsd. > > ufs_rename() is tricky, any change to it requires much testing, so that > we can keep openbsd's -current code quality. so, in a sentence, in case > you're an openbsd user and happen to accidentally have a ufs partition > laying around and to use it for some weird reason, you should test this. > > jokes aside, this kind of user seems to be getting more and more hard > to find everyday, so please don't lose this opportunity to prove you're > one of them. > > -p. > > Index: ufs_vnops.c > ================================================== ================= > RCS file: /cvs/src/sys/ufs/ufs/ufs_vnops.c,v > retrieving revision 1.60 > diff -u -r1.60 ufs_vnops.c > --- ufs_vnops.c 17 Feb 2005 18:07:37 -0000 1.60 > +++ ufs_vnops.c 22 Mar 2005 20:29:02 -0000 > @@ -910,7 +910,7 @@ > /* > * Delete source. There is another race now that everything > * is unlocked, but this doesn't cause any new complications. > - * Relookup() may find a file that is unrelated to the > + * relookup() may find a file that is unrelated to the > * original one, or it may fail. Too bad. > */ > vrele(fvp); > @@ -919,8 +919,10 @@ > if ((fcnp->cn_flags & SAVESTART) == 0) > panic("ufs_rename: lost from startdir"); > fcnp->cn_nameiop = DELETE; > - (void) relookup(fdvp, &fvp, fcnp); > + error = relookup(fdvp, &fvp, fcnp); > vrele(fdvp); > + if (error) > + return (error); > if (fvp == NULL) { > return (ENOENT); > } > @@ -1017,7 +1019,10 @@ > goto bad; > if (xp != NULL) > vput(tvp); > - > + /* > + * Compensate for the reference ufs_checkpath() loses. > + */ > + vref(tdvp); > /* Only tdvp is locked */ > if ((error = ufs_checkpath(ip, dp, tcnp->cn_cred)) != 0) > goto out; > @@ -1171,12 +1176,13 @@ > fcnp->cn_flags |= LOCKPARENT | LOCKLEAF; > if ((fcnp->cn_flags & SAVESTART) == 0) > panic("ufs_rename: lost from startdir"); > - (void) relookup(fdvp, &fvp, fcnp); > + error = relookup(fdvp, &fvp, fcnp); > vrele(fdvp); > - if (fvp != NULL) { > - xp = VTOI(fvp); > - dp = VTOI(fdvp); > - } else { > + if (error) { > + vrele(ap->a_fvp); > + return (error); > + } > + if (fvp == NULL) { > /* > * From name has disappeared. > */ > @@ -1185,6 +1191,10 @@ > vrele(ap->a_fvp); > return (0); > } > + > + xp = VTOI(fvp); > + dp = VTOI(fdvp); > + > /* > * Ensure that the directory entry still exists and has not > * changed while the new name has been entered. If the source is      |
Linear Mode
