Messages in this thread | | | Date | Thu, 22 Apr 1999 09:09:26 -0400 (EDT) | From | Alexander Viro <> | Subject | [PATCH] Several bad bugs in fs/* |
| |
Linus, we have several really bad bugs and I think that they are worth fixing in official tree. I've backported minimal fixes (from the FAT patch) and it would be really nice if you'ld look at it.
In order of decreasing nastiness:
* Buffer overruns in fat_readdirx(). AFAICS they were there since 1.3. Details: fat_readdirx() doesn't do any sanity checks when it reads the first slot of VFAT name. It contains the number of slots occupied by the name. Put there 0xff and you've got a random memory corruption. Another variant (more evil) being: put the name longer than 300 Unicode characters and the thing will happily try to translate it into ASCII. Into the on-stack array. Stack smashing in ring 0... The same effect can be achieved if you'll pick the name of sane length, but with the characters out of mapping. They are translated into 4-character combinations. Longer than 75 and there we go - same stack smashing. Fix: do required sanity checks.
* race in buffer cache. Scenario: 1. we umount filesystem. Suppose that it restores the blocksize upon exit (FAT-derived ones and sysvfs do it). set_blocksize() is called. Notice that we are likely to have a lot of dirty buffers at that moment. set_blocksize() will wait on them, unhash them and leave them on the dirty list. 2. Suppose that we remount the fs immediately. set_blocksize() is called again and buffers are untouched, since they have the right size. They remain unhashed and on the dirty list. 3. Suppose that the same blocks are bread() again. We got two copies - new one (hashed) and old (unhashed). 4. Only now bdflush() wakes up. Finds clean buffer on the dirty list. Does refile_buffer() on it. Which places it on the clean list and rehashes the sucker. Happy, happy, joy, joy. Especially since the next bread() will find the older copy (it's placed into the head of the hash chain). I've actually hit this race testing new VFAT code (Gordon's testsuite does a lot of fast dirtify/umount/mount/dirtify passes), but the same thing happens with the old implementation of FAT-derived filesystems and with sysvfs. Fix: do remove_from_queues() instead of remove_from_hash_queue().
* vfat_rename() does bad things if we are moving a directory opened via different alias. Fix: make vfat_lookup() look for existing aliases, drop them if possible and if they can't be invalidated - return them. Changed ->lookup() type, but in a binary compatible way. Change in filesystems is trivial and looks, erm, obviously correct. It had been tested - FAT patch includes it for quite a while.
* ntfs_lookup() returns positive value in case of error. Yup, missed -. Fix: obvious.
* qnx4_lookup() doesn't return negative dentries - it returns -ENOENT. Not good. Fix: obvious.
* Minor stuff, but annoying and trivial to fix - lots of functions check that directory inode is non-NULL and is indeed a directory. In case of methods it's definitely bogus - checks are done in VFS. Removed.
So there... Patch (against 2.2.7-pre1) attached. Linus, I'ld be very grateful if you could look it through. Folks, please help to test it - stuff should be OK (non-trivial parts are small and already tested), but fsck-up fairy shows up wherever she wants, so... Cheers, Al [unhandled content-type:application/octet-stream] | |