Re: [PATCH] fs: Don't pass stack garbage to filesystem's get_block() in map_bh->b_size

[Dave Kleikamp <shaggy@linux.vnet.ibm.com>]

On Tue, 2009-05-12 at 07:37 -0400, Theodore Ts'o wrote:
> The nobh_truncate_page() function is used by ext2, exofs, and jfs.  Of
> these three, only ext2 and jfs's get_block() function pays attention
> to bh->b_size --- which is normally always the filesystem blocksize
> except when the get_block() function is called by either
> mpage_readpage(), mpage_readpages(), or the direct I/O routines in
> fs/direct_io.c.
> 
> Unfortunately, nobh_truncate_page() does not initialize map_bh before
> calling the filesystem-supplied get_block() function.  So ext2 and jfs
> will try to calculate the number of blocks to map by taking stack
> garbage and shifting it left by inode->i_blkbits.  This should be
> *mostly* harmless (except the filesystem will do some unnneeded work)
> unless the stack garbage is less than filesystem's blocksize, in which
> case maxblocks will be zero, and the attempt to find out whether or
> not the filesystem has a hole at a given logical block will fail, and
> the page cache entry might not get zero'ed out.
> 
> Also if the stack garbage in in map_bh->state happens to have the
> BH_Mapped bit set, there could be an attempt to call readpage() on a
> non-existent page, which could cause nobh_truncate_page() to return an
> error when it should not.
> 
> Fix this by initializing map_bh->state and map_bh->size.

This appears obviously correct.

> Fortunately, it's probably fairly unlikely that ext2 and jfs users
> mount with nobh these days.

Well, jfs doesn't have a nobh mount option.  It always calls the *_nobh
routines.  I don't really have a good excuse why.
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

Acked-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

> Cc: linux-fsdevel@vger.kernel.org
> ---
>  fs/buffer.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index ad01129..1864d0b 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2736,6 +2736,8 @@ has_buffers:
>  		pos += blocksize;
>  	}
> 
> +	map_bh.b_size = blocksize;
> +	map_bh.b_state = 0;
>  	err = get_block(inode, iblock, &map_bh, 0);
>  	if (err)
>  		goto unlock;
-- 
David Kleikamp
IBM Linux Technology Center