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
| ||
|
Message-ID: <20240103110255.7uebqb2iy4jcy6sh@quack3> Date: Wed, 3 Jan 2024 12:02:55 +0100 From: Jan Kara <jack@...e.cz> To: Zhang Yi <yi.zhang@...weicloud.com> Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz, ritesh.list@...il.com, hch@...radead.org, djwong@...nel.org, willy@...radead.org, yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com, wangkefeng.wang@...wei.com Subject: Re: [RFC PATCH v2 03/25] ext4: correct the hole length returned by ext4_map_blocks() On Tue 02-01-24 20:38:56, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@...wei.com> > > In ext4_map_blocks(), if we can't find a range of mapping in the > extents cache, we are calling ext4_ext_map_blocks() to search the real > path and ext4_ext_determine_hole() to determine the hole range. But if > the querying range was partially or completely overlaped by a delalloc > extent, we can't find it in the real extent path, so the returned hole > length could be incorrect. > > Fortunately, ext4_ext_put_gap_in_cache() have already handle delalloc > extent, but it searches start from the expanded hole_start, doesn't > start from the querying range, so the delalloc extent found could not be > the one that overlaped the querying range, plus, it also didn't adjust > the hole length. Let's just remove ext4_ext_put_gap_in_cache(), handle > delalloc and insert adjusted hole extent in ext4_ext_determine_hole(). > > Signed-off-by: Zhang Yi <yi.zhang@...wei.com> > Suggested-by: Jan Kara <jack@...e.cz> Some small comments below. > @@ -4062,6 +4038,66 @@ static int get_implied_cluster_alloc(struct super_block *sb, > return 0; > } > > +/* > + * Determine hole length around the given logical block, first try to > + * locate and expand the hole from the given @path, and then adjust it > + * if it's partially or completely converted to delayed extents. > + */ > +static void ext4_ext_determine_hole(struct inode *inode, > + struct ext4_ext_path *path, > + struct ext4_map_blocks *map) > +{ I'd prefer to keep setting of 'map' inside ext4_ext_map_blocks() to make it more obvious there and just pass map->m_lblk into ext4_ext_determine_hole(). ext4_ext_determine_hole() will just return the length of the extent from lblk that was mapped (i.e. 'len'). Also I'd probably call this function like ext4_ext_determine_insert_hole() to make it more obvious the function modifies the status tree. Otherwise the patch looks good to me. Honza > + ext4_lblk_t hole_start, len; > + struct extent_status es; > + > + hole_start = map->m_lblk; > + len = ext4_ext_find_hole(inode, path, &hole_start); > +again: > + ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start, > + hole_start + len - 1, &es); > + if (!es.es_len) > + goto insert_hole; > + > + /* > + * Found a delayed range in the hole, handle it if the delayed > + * range is before, behind and straddle the queried range. > + */ > + if (map->m_lblk >= es.es_lblk + es.es_len) { > + /* > + * Before the queried range, find again from the queried > + * start block. > + */ > + len -= map->m_lblk - hole_start; > + hole_start = map->m_lblk; > + goto again; > + } else if (in_range(map->m_lblk, es.es_lblk, es.es_len)) { > + /* > + * Straddle the beginning of the queried range, it's no > + * longer a hole, adjust the length to the delayed extent's > + * after map->m_lblk. > + */ > + len = es.es_lblk + es.es_len - map->m_lblk; > + goto out; > + } else { > + /* > + * Partially or completely behind the queried range, update > + * hole length until the beginning of the delayed extent. > + */ > + len = min(es.es_lblk - hole_start, len); > + } > + > +insert_hole: > + /* Put just found gap into cache to speed up subsequent requests */ > + ext_debug(inode, " -> %u:%u\n", hole_start, len); > + ext4_es_insert_extent(inode, hole_start, len, ~0, EXTENT_STATUS_HOLE); > + > + /* Update hole_len to reflect hole size after map->m_lblk */ > + if (hole_start != map->m_lblk) > + len -= map->m_lblk - hole_start; > +out: > + map->m_pblk = 0; > + map->m_len = min_t(unsigned int, map->m_len, len); > +} > > /* > * Block allocation/map/preallocation routine for extents based files > @@ -4179,22 +4215,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > * we couldn't try to create block if create flag is zero > */ > if ((flags & EXT4_GET_BLOCKS_CREATE) == 0) { > - ext4_lblk_t hole_start, hole_len; > - > - hole_start = map->m_lblk; > - hole_len = ext4_ext_determine_hole(inode, path, &hole_start); > - /* > - * put just found gap into cache to speed up > - * subsequent requests > - */ > - ext4_ext_put_gap_in_cache(inode, hole_start, hole_len); > - > - /* Update hole_len to reflect hole size after map->m_lblk */ > - if (hole_start != map->m_lblk) > - hole_len -= map->m_lblk - hole_start; > - map->m_pblk = 0; > - map->m_len = min_t(unsigned int, map->m_len, hole_len); > - > + ext4_ext_determine_hole(inode, path, map); > goto out; > } > > -- > 2.39.2 > -- Jan Kara <jack@...e.com> SUSE Labs, CR
Powered by blists - more mailing lists