Date: Sun, 4 Jun 95 16:20:00 MDT From: terry@cs.weber.edu (Terry Lambert) To: bde@zeta.org.au (Bruce Evans) Cc: hackers@freebsd.org, davidg@root.com Subject: Re: races in ufs_rename() Message-ID: <9506042220.AA23649@cs.weber.edu> In-Reply-To: <199506041518.BAA31044@godzilla.zeta.org.au> from "Bruce Evans" at Jun 5, 95 01:18:16 am
next in thread | previous in thread | raw e-mail | index | archive | help
> > ufs_rename() tries hard to avoid races and deadlocks. I think it fails > to avoid serious races in 2 places: > > 1) After looking up the source and before reaching ufs_rename(). The > parent directory of the source isn't locked (locking might cause > deadlock), so the source directory entry may be moved or deleted. This > need not be serious, but it can cause a panic in the `doingdirectory' > case when the source is unlinked. IN_RENAME was supposed to stop the > source directory entry from being moved and an extra link was supposed > to stop it being deleted, but these tricks are done too late. > > 2) In the `doingdirectory && newparent' case, when ufs_checkpath() is > called, all locks on the target directories are released (hanging on to > them might cause deadlock), so the target directories may be moved or > deleted. I tested this by adding a tsleep() to ufs_checkpath() before > the call to VFS_VGET() and had no difficulty moving the target directory > to a bad place (a subdirectory of the source) while ufs_checkpath() was > sleeping. ufs_rename() should at least check that relookup() of the > target produces the same inode like it does for the source. It needs to do target lookup + lock, then source lookup + lock in that order, instead to close the race. The target should remain locked. A path compare to determine if one is a substring of the other could be done for ordering, and fail for EISDIR. I need to try a couple of things before I can comment further on a cannonically correct fix. I will say that I'm terrifically disappointed with the bottom end interface in ufs actually doing BIO calls at all -- this is a violation of the stacking principles, and locks ufs at the stack bottom no matter what. Terribly bad form. I may rewrite all of that if I get time; I was talking to John (H) recently about his intent vs. the ufs/ffs implementation, and it's strange that there are so man external refs (close to 70?) that aren't generic services only (like bcopy). Terry Lambert terry@cs.weber.edu --- Any opinions in this posting are my own and not those of my present or previous employers.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9506042220.AA23649>