[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-fsdevel
Subject:    [PATCH] Several bad bugs in fs/*
From:       Alexander Viro <viro () math ! psu ! edu>
Date:       1999-04-22 13:09:26
[Download RAW message or body]


	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

["aliases-path-14.gz" (APPLICATION/octet-stream)]

[prev in list] [next in list] [prev in thread] [next in thread]