Re: Bug in fs/reiserfs/file.c

[Hans Reiser <reiser@namesys.com>]

Thanks Neil, I'll have vs evaluate and fix this.

Hans

Neil Brown wrote:

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