ext4: fix end of leaf partial cluster handling

The fix in commit ad6599ab3a ("ext4: fix premature freeing of
partial clusters split across leaf blocks"), intended to avoid
dereferencing an invalid extent pointer when determining whether a
partial cluster should be freed, wasn't quite good enough.  Assure that
at least one extent remains at the start of the leaf once the hole has
been punched.  Otherwise, the pointer to the extent to the right of the
hole will be invalid and a partial cluster will be incorrectly freed.

Set partial_cluster to 0 when we can tell we've hit the left edge of
the punched region within the leaf.  This prevents incorrect freeing
of a partial cluster when ext4_ext_rm_leaf is called one last time
during extent tree traversal after the punched region has been removed.

Adjust comments to reflect code changes and a correction.  Remove a bit
of dead code.

Signed-off-by: Eric Whitney <enwlinux@gmail.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
This commit is contained in:
Eric Whitney 2014-11-23 00:58:11 -05:00 committed by Theodore Ts'o
parent f4226d9ea4
commit 5bf4376065

View file

@ -2574,15 +2574,16 @@ static int ext4_remove_blocks(handle_t *handle, struct inode *inode,
/* /*
* ext4_ext_rm_leaf() Removes the extents associated with the * ext4_ext_rm_leaf() Removes the extents associated with the
* blocks appearing between "start" and "end", and splits the extents * blocks appearing between "start" and "end". Both "start"
* if "start" and "end" appear in the same extent * and "end" must appear in the same extent or EIO is returned.
* *
* @handle: The journal handle * @handle: The journal handle
* @inode: The files inode * @inode: The files inode
* @path: The path to the leaf * @path: The path to the leaf
* @partial_cluster: The cluster which we'll have to free if all extents * @partial_cluster: The cluster which we'll have to free if all extents
* has been released from it. It gets negative in case * has been released from it. However, if this value is
* that the cluster is still used. * negative, it's a cluster just to the right of the
* punched region and it must not be freed.
* @start: The first block to remove * @start: The first block to remove
* @end: The last block to remove * @end: The last block to remove
*/ */
@ -2730,8 +2731,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
sizeof(struct ext4_extent)); sizeof(struct ext4_extent));
} }
le16_add_cpu(&eh->eh_entries, -1); le16_add_cpu(&eh->eh_entries, -1);
} else if (*partial_cluster > 0) }
*partial_cluster = 0;
err = ext4_ext_dirty(handle, inode, path + depth); err = ext4_ext_dirty(handle, inode, path + depth);
if (err) if (err)
@ -2750,20 +2750,18 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
/* /*
* If there's a partial cluster and at least one extent remains in * If there's a partial cluster and at least one extent remains in
* the leaf, free the partial cluster if it isn't shared with the * the leaf, free the partial cluster if it isn't shared with the
* current extent. If there's a partial cluster and no extents * current extent. If it is shared with the current extent
* remain in the leaf, it can't be freed here. It can only be * we zero partial_cluster because we've reached the start of the
* freed when it's possible to determine if it's not shared with * truncated/punched region and we're done removing blocks.
* any other extent - when the next leaf is processed or when space
* removal is complete.
*/ */
if (*partial_cluster > 0 && eh->eh_entries && if (*partial_cluster > 0 && ex >= EXT_FIRST_EXTENT(eh)) {
(EXT4_B2C(sbi, ext4_ext_pblock(ex) + ex_ee_len - 1) != pblk = ext4_ext_pblock(ex) + ex_ee_len - 1;
*partial_cluster)) { if (*partial_cluster != (long long) EXT4_B2C(sbi, pblk)) {
int flags = get_default_free_blocks_flags(inode); ext4_free_blocks(handle, inode, NULL,
EXT4_C2B(sbi, *partial_cluster),
ext4_free_blocks(handle, inode, NULL, sbi->s_cluster_ratio,
EXT4_C2B(sbi, *partial_cluster), get_default_free_blocks_flags(inode));
sbi->s_cluster_ratio, flags); }
*partial_cluster = 0; *partial_cluster = 0;
} }