linux-ext4 - Re: [PATCH] ext4: prevent right-shifting extents beyond EXT_MAX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180410144328.GB7493@magnolia>
Date:   Tue, 10 Apr 2018 07:43:28 -0700
From:   "Darrick J. Wong" <darrick.wong@...cle.com>
To:     Eric Biggers <ebiggers3@...il.com>
Cc:     linux-ext4@...r.kernel.org, "Theodore Ts'o" <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] ext4: prevent right-shifting extents beyond
 EXT_MAX_BLOCKS

On Tue, Apr 10, 2018 at 12:01:04AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@...gle.com>
> 
> During the "insert range" fallocate operation, extents starting at the
> range offset are shifted "right" (to a higher file offset) by the range
> length.  But, as shown by syzbot, it's not validated that this doesn't
> cause extents to be shifted beyond EXT_MAX_BLOCKS.  In that case
> ->ee_block can wrap around, corrupting the extent tree.
> 
> Fix it by returning an error if the space between the end of the last
> extent and EXT4_MAX_BLOCKS is smaller than the range being inserted.
> 
> This bug can be reproduced by running the following commands when the
> current directory is on an ext4 filesystem with a 4k block size:
> 
>         fallocate -l 8192 file
>         fallocate --keep-size -o 0xfffffffe000 -l 4096 -n file
>         fallocate --insert-range -l 8192 file
> 
> Then after unmounting the filesystem, e2fsck reports corruption.

Could you please wrap this up into a xfstest for regression testing?

--D

> Reported-by: syzbot+06c885be0edcdaeab40c@...kaller.appspotmail.com
> Fixes: 331573febb6a ("ext4: Add support FALLOC_FL_INSERT_RANGE for fallocate")
> Cc: <stable@...r.kernel.org> # v4.2+
> Signed-off-by: Eric Biggers <ebiggers@...gle.com>
> ---
>  fs/ext4/extents.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 0a7315961bac6..c969275ce3ee7 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5329,8 +5329,9 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
>  	stop = le32_to_cpu(extent->ee_block);
>  
>         /*
> -	 * In case of left shift, Don't start shifting extents until we make
> -	 * sure the hole is big enough to accommodate the shift.
> +	* For left shifts, make sure the hole on the left is big enough to
> +	* accommodate the shift.  For right shifts, make sure the last extent
> +	* won't be shifted beyond EXT_MAX_BLOCKS.
>  	*/
>  	if (SHIFT == SHIFT_LEFT) {
>  		path = ext4_find_extent(inode, start - 1, &path,
> @@ -5350,9 +5351,14 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
>  
>  		if ((start == ex_start && shift > ex_start) ||
>  		    (shift > start - ex_end)) {
> -			ext4_ext_drop_refs(path);
> -			kfree(path);
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +	} else {
> +		if (shift > EXT_MAX_BLOCKS -
> +		    (stop + ext4_ext_get_actual_len(extent))) {
> +			ret = -EINVAL;
> +			goto out;
>  		}
>  	}
>  
> -- 
> 2.17.0
>