Messages in this thread | | | Date | Sun, 29 Feb 2004 13:14:25 +0000 | From | viro@parcelfa ... | Subject | Re: 2.6.4-rc1 oops on HPFS filesystem file rename |
| |
On Sun, Feb 29, 2004 at 12:31:30PM +0100, Maurice van der Stee wrote: > I think I tested most of the basic file system operations, rename, > move, copy, edit, truncate etc. and found no problems with the second > patch applied..
Actually, I've found a problem with the original - race between write_inode() and cross-directory rename(), of all things. Mind testing the patchset on ftp.linux.org.uk/pub/people/viro/HPFS*? First two patches were posted in this thread, the rest goes on top of them.
Race in question is funny, BTW - write_inode() might have to find and update directory entry in the parent, so it gets the parent inode and locks it (that's what hpfs-private lock was for - it gave exclusion between directory modifications and write_inode() directory entry update; directory entries can move on insertion/removal, so we need to make sure that it won't happen during that updating). The trouble being, it did not make sure that we lock the right directory - if rename() had moved the file in question while we were trying to lock the parent, we would end up with very confused write_inode().
Similar race exists for unlink() vs. write_inode() - the latter checks ->i_nlink, but there is no warranty that it won't become 0 between the check and locking the parent. Ditto for rmdir() and overwriting rename().
The fix is simple: we turn semaphore into sempahore + rwsem and 1) on mkdir/mknod/create/symlink grab rwsem on parent for writing 2) on rmdir/unlink/rename we grab semaphore on victim and object being moved, then grab rwsem on parent(s) for writing. 3) on write_inode we grab semaphore on inode, then check that it has non-zero ->i_nlink, get parent and grab rwsem on parent for reading.
Exclusion already provided by VFS makes order in rename() (there we might get two semaphores and two rwsems) a non-issue - we already have all ->rename() on given fs serialized. Order among semaphores and among rwsems, that is - semaphores are always taken first.
Aside of that, patchset switches from use of iget() to use of iget_locked(), which allows to kill the horrors in hpfs_lock_iget(), and does general cleanup.
Unless there are complaints and bug reports, it will go to Linus in a couple of days. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |