Re: [PATCH v1] ext2: fix datatype of block number in ext2_xattr_set2()

[Jan Kara]

On Tue 15-08-23 12:03:40, Georg Ottinger wrote:
> I run a small server that uses external hard drives for backups. The
> backup software I use uses ext2 filesystems with 4KiB block size and
> the server is running SELinux and therefore relies on xattr. I recently
> upgraded the hard drives from 4TB to 12TB models. I noticed that after
> transferring some TBs I got a filesystem error "Freeing blocks not in
> datazone - block = 18446744071529317386, count = 1" and the backup
> process stopped. Trying to fix the fs with e2fsck resulted in a
> completely corrupted fs. The error probably came from ext2_free_blocks(),
> and because of the large number 18e19 this problem immediately looked
> like some kind of integer overflow. Whereas the 4TB fs was about 1e9
> blocks, the new 12TB is about 3e9 blocks. So, searching the ext2 code,
> I came across the line in fs/ext2/xattr.c:745 where ext2_new_block()
> is called and the resulting block number is stored in the variable block
> as an int datatype. If a block with a block number greater than
> INT32_MAX is returned, this variable overflows and the call to
> sb_getblk() at line fs/ext2/xattr.c:750 fails, then the call to
> ext2_free_blocks() produces the error.
> 
> Signed-off-by: Georg Ottinger <g.ottinger@xxxxxx>

Yeah, definitely looks like a bug. Thanks for the fix! I've added it to my
tree.

BTW I'm not sure which kernel (and with which config) you are using but in
most distribution kernels, ext2 filesystem driver is not used and instead
ext4 driver is used for handling even ext2 filesystems. Just mentioning
that in case your fs corruption is in fact related to ext4 driver...

								Honza

> ---
>  fs/ext2/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> index 8906ba479..5e13f7ea1 100644
> --- a/fs/ext2/xattr.c
> +++ b/fs/ext2/xattr.c
> @@ -742,7 +742,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
>  			/* We need to allocate a new block */
>  			ext2_fsblk_t goal = ext2_group_first_block_no(sb,
>  						EXT2_I(inode)->i_block_group);
> -			int block = ext2_new_block(inode, goal, &error);
> +			ext2_fsblk_t block = ext2_new_block(inode, goal, &error);
>  			if (error)
>  				goto cleanup;
>  			ea_idebug(inode, "creating block %d", block);
> --
> 2.17.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR