linux-ext4 - Re: [PATCH v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK

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: <20170914120229.GA22886@quack2.suse.cz>
Date:   Thu, 14 Sep 2017 14:02:29 +0200
From:   Jan Kara <jack@...e.cz>
To:     Andreas Gruenbacher <agruenba@...hat.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-xfs@...r.kernel.org, Jan Kara <jack@...e.cz>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v2 4/4] ext4: Switch to iomap for SEEK_HOLE / SEEK_DATA

On Thu 14-09-17 11:50:47, Andreas Gruenbacher wrote:
> @@ -3422,9 +3422,38 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  	map.m_lblk = first_block;
>  	map.m_len = last_block - first_block + 1;
>  
> -	if (!(flags & IOMAP_WRITE)) {
> +	if (flags & IOMAP_REPORT) {
>  		ret = ext4_map_blocks(NULL, inode, &map, 0);
> -	} else {
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret == 0) {
> +			ext4_lblk_t end = map.m_lblk + map.m_len - 1;
> +			struct extent_status es;
> +
> +			ext4_es_find_delayed_extent_range(inode, map.m_lblk, end, &es);
> +
> +			if (!es.es_len || es.es_lblk > end) {
> +				/* entire range is a hole */
> +			} else if (es.es_lblk > map.m_lblk) {
> +				/* range starts with a hole */
> +				map.m_len = es.es_lblk - map.m_lblk;
> +			} else {
> +				ext4_lblk_t offs = 0;
> +
> +				if (es.es_lblk < map.m_lblk)
> +					offs = map.m_lblk - es.es_lblk;
> +				map.m_lblk = es.es_lblk + offs;
> +				map.m_pblk = ext4_es_pblock(&es) + offs;
> +				map.m_len = es.es_len - offs;
> +				if (ext4_es_is_unwritten(&es))
> +					map.m_flags |= EXT4_MAP_UNWRITTEN;

This is not possible. If there is unwritten extent at map->m_lblk, then
ext4_map_blocks() could not have possibly returned 0. So just delete this
condition and set map.m_pblk to 0 since delalloc extent has no physical
possition.

> +				if (ext4_es_is_delayed(&es))
> +					delalloc = true;

Also ext4_es_is_delayed(&es) is guaranteed to be true since you've looked
it up by ext4_es_find_delayed_extent_range(). So just set delalloc = true
unconditionally.

Otherwise the patch looks good so after fixing up these small issues feel
free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR