linux-kernel - Re: [PATCH] ext2: Handle fiemap on empty files to prevent EINVAL

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: <ve7pt2nbw6qmdoy2i6dzvtlougxjcl6eye73kjysomhxx4vans@24vgczp5ospk>
Date: Thu, 12 Jun 2025 12:29:16 +0200
From: Jan Kara <jack@...e.cz>
To: Wei Gao <wegao@...e.com>
Cc: linux-kernel@...r.kernel.org, jack@...e.com, 
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH] ext2: Handle fiemap on empty files to prevent EINVAL

On Thu 12-06-25 10:28:55, Wei Gao wrote:
> Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len,
> i_size_read(inode));", When inode->i_size was 0 (for an empty file), this
> would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could
> then result in an -EINVAL error, even for valid queries on empty files.
> The new validation logic directly references ext4_fiemap_check_ranges.
> 
> Link: https://github.com/linux-test-project/ltp/issues/1246
> Signed-off-by: Wei Gao <wegao@...e.com>

Thanks for the fix. Some comments below:

> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 30f8201c155f..e5cc61088f21 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -895,10 +895,30 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  		u64 start, u64 len)
>  {
>  	int ret;
> +	u64 maxbytes;
>  
>  	inode_lock(inode);
> -	len = min_t(u64, len, i_size_read(inode));

So I agree this 'len' reduction to i_size looks bogus and can be removed.
But why are you adding those maxbytes checks when they are done inside
iomap_fiemap() shorly later?

								Honza


> +	maxbytes = inode->i_sb->s_maxbytes;
> +
> +	if (len == 0) {
> +		ret = -EINVAL;
> +		goto unlock_inode;
> +	}
> +
> +	if (start > maxbytes) {
> +		ret = -EFBIG;
> +		goto unlock_inode;
> +	}
> +
> +	/*
> +	 * Shrink request scope to what the fs can actually handle.
> +	 */
> +	if (len > maxbytes || (maxbytes - len) < start)
> +		len = maxbytes - start;
> +
>  	ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops);
> +
> +unlock_inode:
>  	inode_unlock(inode);
>  
>  	return ret;
> -- 
> 2.49.0
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR