Re: + fix-possible-page_cache_shift-overflows.patch added to -mm tree

[Oleg Drokin <green@linuxhacker.ru>]

Hello!

On Wed, Nov 23, 2005 at 01:22:16PM -0800, akpm@osdl.org wrote:

> - reiserfs map_block_for_writepage() takes an unsigned long for the block -
>   it should take sector_t.  (It'll fail for huge filesystems with
>   blocksize<PAGE_CACHE_SIZE)

reiserfs is almost never used with blocksize other than 4k (And support
for block size smaller than 4k appeared only around 3 years ago).
And the only supported architectures that have PAGE_CACHE_SIZE bigger
than 4k are 64bit ones, so they are safe still.
Also sector_t is not very suitable for the job, because it is still 32 bit
with common config settings (no large block devices support).
So before proposing, even cosmetic, patch here, I want to understand - do we
allow files to be bigger than (u64)((sector_t)-1)*fs_block_size?
If we do, then we are in trouble even with sector_t. (And I do not see
what would prevent us to have a file that is bigger than 4T on ext2 with
512 byte blocksize, for example).

Also thanks to Andrews, I now see another overflow in reiserfs that should
lead to data corruptions with files that are bigger than 4G under certain
circumstances when using mmap.
In map_block_for_writepage():
        loff_t byte_offset = (block << inode->i_sb->s_blocksize_bits) + 1;

Please consider patch below:

--- linux-2.6.14/fs/reiserfs/inode.c.orig	2005-11-24 13:26:26.000000000 +0200
+++ linux-2.6.14/fs/reiserfs/inode.c	2005-11-24 13:28:35.000000000 +0200
@@ -2194,7 +2194,7 @@ static int map_block_for_writepage(struc
 	INITIALIZE_PATH(path);
 	int pos_in_item;
 	int jbegin_count = JOURNAL_PER_BALANCE_CNT;
-	loff_t byte_offset = (block << inode->i_sb->s_blocksize_bits) + 1;
+	loff_t byte_offset = ((loff_t)block << inode->i_sb->s_blocksize_bits)+1;
 	int retval;
 	int use_get_block = 0;
 	int bytes_copied = 0;


Bye,
    Oleg