diff mbox

[4/4] ext4: Fix different block exchange issue in EXT4_IOC_MOVE_EXT

Message ID 4AADFC1A.7020609@rs.jp.nec.com
State Accepted, archived
Headers show

Commit Message

Akira Fujita Sept. 14, 2009, 8:17 a.m. UTC 
ext4: Fix different block exchange issue in EXT4_IOC_MOVE_EXT

From: Akira Fujita <a-fujita@rs.jp.nec.com>

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　<bergwolf@gmail.com>　
Signed-off-by: Akira Fujita <a-fujita@rs.jp.nec.com>
---

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

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Theodore Ts'o Sept. 16, 2009, 6:20 p.m. UTC | #1 
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 <a-fujita@rs.jp.nec.com>
> 
> 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　<bergwolf@gmail.com>　
> Signed-off-by: Akira Fujita <a-fujita@rs.jp.nec.com>

Applied, thanks.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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) {