Messages in this thread | | | Date | Sun, 22 Apr 2001 00:16:12 +0200 | From | Roman Zippel <> | Subject | Re: Races in affs_unlink(), affs_rmdir() and affs_rename() |
| |
Hi,
Alexander Viro wrote:
> unlink("/B/b") locks /B, removes "b" and unlocks /B. Then it calls > affs_remove_link(), which blocks. > > unlink("/A/a") locks /A, removes "a" and unlocks /A. Then it calls > affs_remove_link(). Which locks /B, renames removed entry into "b", > removes old "b" and inserts renamed "a" into /B. > > The rest is irrelevant - we're already in it.
Thanks for finding that one, but it should be easy to fix. I can remove the parent pointer in aff_remove_hash and check for that before I try to rename that entry.
> Since you don't lock /B for affs_empty_dir(), you can hit the > window between removing old /B/a and inserting renamed /A/a into /B. > Notice that VFS _does_ lock /B (->i_zombie), but affs_remove_link() > for /A/a doesn't even look at it.
I thought about that one and I know it should be locked. The reason I don't do right now is, that affs supports hardlinks to dirs. The problem are especially recursive links, e.g.:
mkdir A ln A A/B rm A/B
This is possible with affs, but will already deadlock in vfs.
mkdir A mkdir A/B ln A A/B/C rm A/B/C/A & rm A/B/C & rm A/B
Every rm already takes the hash lock of the parent and then I can't simply also take the hash lock of the dir itself. What I actually want to do is to insert a reverse is_subdir() check before taking the lock. On the other hand I was thinking whether I should allow links to dirs at all and just show them as empty/readonly dirs. For 2.4 that's probably safer, as it would require a lot of locking changes in vfs and the other fs to support this properly, particularly moving most of the locking from vfs into the fs.
bye, Roman - 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/
| |