2009-09-14 08:19:30

by Akira Fujita

[permalink] [raw]
Subject: [PATCH 4/4]ext4: Fix different block exchange issue in EXT4_IOC_MOVE_EXT

ext4: Fix different block exchange issue in EXT4_IOC_MOVE_EXT

From: Akira Fujita <[email protected]>

If logical block offset of original file which is passed to
EXT4_IOC_MOVE_EXT is different from donor file's,
a calculation error occurs in ext4_calc_swap_extents(),
therefore wrong block is exchanged between original file and donor file.
As a result, we hit ext4_error() in check_block_validity().
To detect the logical offset difference in EXT4_IOC_MOVE_EXT,
add checks to mext_calc_swap_extents() and handle it as error,
since data exchange must be done between the same blocks in EXT4_IOC_MOVE_EXT.

Reported-by: Peng Tao <[email protected]> 
Signed-off-by: Akira Fujita <[email protected]>
---

fs/ext4/move_extent.c | 46 +++++++++++++++++++++++++++++++++++++---------
1 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 7c26c74..a22371f 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -597,8 +597,10 @@ out:
* @orig_off: block offset of original inode
* @donor_off: block offset of donor inode
* @max_count: the maximun length of extents
+ *
+ * Return 0 on success, or a negative error value on failure.
*/
-static void
+static int
mext_calc_swap_extents(struct ext4_extent *tmp_dext,
struct ext4_extent *tmp_oext,
ext4_lblk_t orig_off, ext4_lblk_t donor_off,
@@ -607,6 +609,19 @@ mext_calc_swap_extents(struct ext4_extent *tmp_dext,
ext4_lblk_t diff, orig_diff;
struct ext4_extent dext_old, oext_old;

+ BUG_ON(orig_off != donor_off);
+
+ /* original and donor extents have to cover the same block offset */
+ if (orig_off < le32_to_cpu(tmp_oext->ee_block) ||
+ le32_to_cpu(tmp_oext->ee_block) +
+ ext4_ext_get_actual_len(tmp_oext) - 1 < orig_off)
+ return -ENODATA;
+
+ if (orig_off < le32_to_cpu(tmp_dext->ee_block) ||
+ le32_to_cpu(tmp_dext->ee_block) +
+ ext4_ext_get_actual_len(tmp_dext) - 1 < orig_off)
+ return -ENODATA;
+
dext_old = *tmp_dext;
oext_old = *tmp_oext;

@@ -634,6 +649,8 @@ mext_calc_swap_extents(struct ext4_extent *tmp_dext,

copy_extent_status(&oext_old, tmp_dext);
copy_extent_status(&dext_old, tmp_oext);
+
+ return 0;
}

/**
@@ -690,8 +707,10 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode,
dext = donor_path[depth].p_ext;
tmp_dext = *dext;

- mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off,
+ err = mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off,
donor_off, count);
+ if (err)
+ goto out;

/* Loop for the donor extents */
while (1) {
@@ -760,9 +779,10 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode,
}
tmp_dext = *dext;

- mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off,
- donor_off,
- count - replaced_count);
+ err = mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off,
+ donor_off, count - replaced_count);
+ if (err)
+ goto out;
}

out:
@@ -1244,11 +1264,15 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
ext_cur = holecheck_path[depth].p_ext;

/*
- * Get proper extent whose ee_block is beyond block_start
- * if block_start was within the hole.
+ * Get proper starting location of block replacement if block_start was
+ * within the hole.
*/
if (le32_to_cpu(ext_cur->ee_block) +
ext4_ext_get_actual_len(ext_cur) - 1 < block_start) {
+ /*
+ * The hole exists between extents or the tail of
+ * original file.
+ */
last_extent = mext_next_extent(orig_inode,
holecheck_path, &ext_cur);
if (last_extent < 0) {
@@ -1261,8 +1285,12 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
ret1 = last_extent;
goto out;
}
- }
- seq_start = block_start;
+ seq_start = le32_to_cpu(ext_cur->ee_block);
+ } else if (le32_to_cpu(ext_cur->ee_block) > block_start)
+ /* The hole exists at the beginning of original file. */
+ seq_start = le32_to_cpu(ext_cur->ee_block);
+ else
+ seq_start = block_start;

/* No blocks within the specified range. */
if (le32_to_cpu(ext_cur->ee_block) > block_end) {


2009-09-16 18:20:56

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 4/4]ext4: Fix different block exchange issue in EXT4_IOC_MOVE_EXT

On Mon, Sep 14, 2009 at 05:17:30PM +0900, Akira Fujita wrote:
> ext4: Fix different block exchange issue in EXT4_IOC_MOVE_EXT
>
> From: Akira Fujita <[email protected]>
>
> If logical block offset of original file which is passed to
> EXT4_IOC_MOVE_EXT is different from donor file's,
> a calculation error occurs in ext4_calc_swap_extents(),
> therefore wrong block is exchanged between original file and donor file.
> As a result, we hit ext4_error() in check_block_validity().
> To detect the logical offset difference in EXT4_IOC_MOVE_EXT,
> add checks to mext_calc_swap_extents() and handle it as error,
> since data exchange must be done between the same blocks in EXT4_IOC_MOVE_EXT.
>
> Reported-by: Peng Tao<[email protected]>
> Signed-off-by: Akira Fujita <[email protected]>

Applied, thanks.

- Ted