linux-ext4 - Re: [PATCH 1/4] ext4: fix trim length underflow with small trim length.

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: <alpine.LFD.2.00.1107011140400.4168@dhcp-27-109.brq.redhat.com>
Date:	Fri, 1 Jul 2011 11:45:17 +0200 (CEST)
From:	Lukas Czerner <lczerner@...hat.com>
To:	Tao Ma <tm@....ma>
cc:	linux-ext4@...r.kernel.org, tytso@....edu
Subject: Re: [PATCH 1/4] ext4: fix trim length underflow with small trim
 length.

On Thu, 30 Jun 2011, Tao Ma wrote:

> From: Tao Ma <boyu.mt@...bao.com>
> 
> In 0f0a25b, we adjust 'len' with s_first_data_block - start, but
> it could underflow in case blocksize=1K, fstrim_range.len=512 and
> fstrim_range.start = 0. In this case, when we run the code:
> len -= first_data_blk - start; len will be underflow to -1ULL.
> In the end, although we are safe that last_group check later will limit
> the trim to the whole volume, but that isn't what the user really want.
> 
> So this patch fix it. It also adds the check for 'start' like ext3 so that
> we can break immediately if the start is invalid.

Hi Tao,

thanks for the resend!

> 
> Signed-off-by: Tao Ma <boyu.mt@...bao.com>
> ---
>  fs/ext4/mballoc.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 6ed859d..2336424 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4904,6 +4904,9 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
>  
>  	if (unlikely(minlen > EXT4_BLOCKS_PER_GROUP(sb)))
>  		return -EINVAL;
> +	if (start >= ext4_blocks_count(EXT4_SB(sb)->s_es) ||
> +	    start + len <= first_data_blk)
> +		goto out;

We should really return -EINVAL in case that start is beyond the
filesystem. However we can not return -EINVAL in case that start+len is
before the first data block, because it would require user to know fs
internals.

So simply doing this, should be enough:

	if (start + len <= first_data_blk)
		goto out;

and the code later

	if (first_group > last_group)
		return -EINVAL;

will handle the rest.

Thanks!
-Lukas


>  	if (start < first_data_blk) {
>  		len -= first_data_blk - start;
>  		start = first_data_blk;
> @@ -4952,5 +4955,6 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
>  	}
>  	range->len = trimmed * sb->s_blocksize;
>  
> +out:
>  	return ret;
>  }
> 

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html