From: Neil Brown <neilb@suse.de>
To: reiserfs-dev@namesys.com, linux-kernel@vger.kernel.org
Date: Tue, 28 Feb 2006 12:29:33 +1100
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Message-ID: <17411.42877.14972.748051@cse.unsw.edu.au>
Subject: Bug in fs/reiserfs/file.c
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1990
Lines: 69


In fs/reiserfs/file.c, in reiserfs_file_write, at line 1400 in
2.6.16-rc2-mm1 we have

		size_t blocks_to_allocate;	/* how much blocks we need to allocate for this iteration */

size_t is an unsigned type.

Later (line 1467) we have code like:

		blocks_to_allocate =
		    reiserfs_prepare_file_region_for_write(inode, pos,
							   num_pages,
							   write_bytes,
							   prepared_pages);
		if (blocks_to_allocate < 0) {
			res = blocks_to_allocate;
			reiserfs_release_claimed_blocks(inode->i_sb,
							num_pages <<
							(PAGE_CACHE_SHIFT -
							 inode->i_blkbits));
			break;
		}


Spot the bug.... reiserfs_prepare_file_region_for_write can return a
negative error status, but blocks_to_allocate won't store it, and
things go wrong.

The actual result if reiserfs_prepare_file_region_for_write returns
negative is that a subsequent call to 
			    reiserfs_allocate_blocks_for_region(&th, inode, pos,
								num_pages,
								write_bytes,
								prepared_pages,
								blocks_to_allocate);
trys to kmalloc an enormous amount of memory
	allocated_blocks = kmalloc((blocks_to_allocate + will_prealloc) *
				   sizeof(b_blocknr_t), GFP_NOFS);

and fails so
		if (res) {
			reiserfs_unprepare_pages(prepared_pages, num_pages);
			break;
		}

which tries to unlock the pages in prepared_pages. But
reiserfs_prepare_file_region_for_write didn't leave any locked pages
in their (due to it's failure) and try_to_free_buffers BUGs out.

The "obvious" fix it to change the 'size_t' to 'ssize_t', but I'll
leave to to reiserfs-dev to create and submit a patch....


As an aside, 
  info gcc
tells me that '-W' will cause a warning when

        * An unsigned value is compared against zero with `<' or `<='.

It doesn't :-(

NeilBrown
-
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/