d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fixed some races and misleading comments in ufs_rename().

Browse Source 
1. When a directory is renamed to an existing (empty) directory,
it is possible for the target vnode to become the source vnode
underneath you (because another process may complete the same
rename).  It was assumed that this can't happen, and the bogus
errno EINVAL was returned.  This was fairly harmless.

Fix: return ENOENT instead, as if the source directory was renamed
a little earlier.

2. The same metamorphosis is possible for non-directories.  It was
assumed that this can't happen, and the code for handling "just
removing a link name" happened to be used.  This would have worked
except for fatal bugs in the link name removal - the link name was
assumed to still be there, and a null pointer was followed.

Fix: check the result of relookup().  This fixes PR 1930.

Notes:

(a) POSIX seems to say that removing link names shall have no effect.
BSD (4.4Lite2 at least) does something reasonable instead.

(b) The relookup() may find a file unrelated to the original.
Removing this isn't correct.  Consider 3 existing files A, B and
C, and concurrent renames: AB = rename(A, B), another AB, and
CA = rename("c", "a").  If rename() is atomic, then only the
following results are possible:

	AB, AB (fails), CA: A = original C, B = original A, C = gone
	AB, CA, AB:         A = gone,       B = original C, C = gone
	CA, AB, AB (fails): A = gone,       B = original C, C = gone

but ufs_rename() can give:

	A,AB,CA,B (sorta):  A = gone,       B = original A, C = gone

This usually doesn't matter, since getting into a race is usually
an error.
---

These fixes should be in 2.1.6 and 2.2.
This commit is contained in:
Bruce Evans 1996-11-04 16:05:51 +00:00
parent 5def50038a
commit 206e5b7e27
1 changed files with 30 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
sys/ufs/ufs/ufs_vnops.c
View File

@ -36,7 +36,7 @@
* SUCH DAMAGE.
*
* @(#)ufs_vnops.c 8.10 (Berkeley) 4/1/94
* $Id: ufs_vnops.c,v 1.40 1996/09/03 14:25:27 bde Exp $
* $Id: ufs_vnops.c,v 1.41 1996/09/19 18:21:32 nate Exp $
*/
#include "opt_quota.h"
@ -842,17 +842,30 @@ ufs_rename(ap)
return (error);
}
/*
* Check if just deleting a link name.
*/
if (tvp && ((VTOI(tvp)->i_flags & (IMMUTABLE | APPEND)) ||
(VTOI(tdvp)->i_flags & APPEND))) {
error = EPERM;
goto abortit;
}
/*
* Check if just deleting a link name or if we've lost a race.
* If another process completes the same rename after we've looked
* up the source and have blocked looking up the target, then the
* source and target inodes may be identical now although the
* names were never linked.
*/
if (fvp == tvp) {
if (fvp->v_type == VDIR) {
error = EINVAL;
/*
* Linked directories are impossible, so we must
* have lost the race. Pretend that the rename
* completed before the lookup.
*/
#ifdef UFS_RENAME_DEBUG
printf("ufs_rename: fvp == tvp for directories\n");
#endif
error = ENOENT;
goto abortit;
}
@ -861,7 +874,12 @@ ufs_rename(ap)
vput(tdvp);
vput(tvp);
/* Delete source. */
/*
* 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
* original one, or it may fail. Too bad.
*/
vrele(fdvp);
vrele(fvp);
fcnp->cn_flags &= ~MODMASK;
@ -873,6 +891,12 @@ ufs_rename(ap)
error = relookup(fdvp, &fvp, fcnp);
if (error == 0)
vrele(fdvp);
if (fvp == NULL) {
#ifdef UFS_RENAME_DEBUG
printf("ufs_rename: from name disappeared\n");
#endif
return (ENOENT);
}
return (VOP_REMOVE(fdvp, fvp, fcnp));
}
error = VOP_LOCK(fvp);