diff mbox

ext4: fix extent tree corruption that incurred by hole punch [V2]

Message ID CAJSVwFM3vq9wsq0Mn5L1bh-WHoP1g3VMJAOg_ruavZFednsjVw@mail.gmail.com
State Accepted, archived
Headers show

Commit Message

Forrest Liu Dec. 13, 2012, 3:32 p.m. UTC
When depth of extent tree is greater than 1, logical start value
of interior node didn't updated correctly in ext4_ext_rm_idx.

Signed-off-by: Forrest Liu <forrestl@synology.com>
---
 fs/ext4/extents.c |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)


@@ -2557,7 +2571,7 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,
 	/* if this leaf is free, then we should
 	 * remove it from index block above */
 	if (err == 0 && eh->eh_entries == 0 && path[depth].p_bh != NULL)
-		err = ext4_ext_rm_idx(handle, inode, path + depth);
+		err = ext4_ext_rm_idx(handle, inode, path, depth);

 out:
 	return err;
@@ -2760,7 +2774,7 @@ again:
 				/* index is empty, remove it;
 				 * handle must be already prepared by the
 				 * truncatei_leaf() */
-				err = ext4_ext_rm_idx(handle, inode, path + i);
+				err = ext4_ext_rm_idx(handle, inode, path, i);
 			}
 			/* root level has p_bh == NULL, brelse() eats this */
 			brelse(path[i].p_bh);

Comments

Ashish Sangwan Dec. 17, 2012, 4:25 a.m. UTC | #1
On Thu, Dec 13, 2012 at 9:02 PM, Forrest Liu <forrestl@synology.com> wrote:
> When depth of extent tree is greater than 1, logical start value
> of interior node didn't updated correctly in ext4_ext_rm_idx.
>
> Signed-off-by: Forrest Liu <forrestl@synology.com>
> ---


Patch seems ok to me.
Reviewed-by: Ashish Sangwan <ashishsangwan2@gmail.com>
--
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
Theodore Ts'o Dec. 20, 2012, 5:39 a.m. UTC | #2
On Thu, Dec 13, 2012 at 11:32:22PM +0800, Forrest Liu wrote:
> When depth of extent tree is greater than 1, logical start value
> of interior node didn't updated correctly in ext4_ext_rm_idx.
> 
> Signed-off-by: Forrest Liu <forrestl@synology.com>

Applied.  

BTW, your reproduction case also results in a file system which isn't
noticed as being broken by e2fsck.  Eric's patch "e2fsck: Fix
incorrect interior node logical start values" fixes e2fsck so it
handles this.  Unfortunately applying his patch seems to uncover a bug
in e2fsck when clearing a bad extent node (f_extent_bad_node) which we
need to fix so the regression test suite is passing.

     	       	   	      	   	    - 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
Forrest Liu Dec. 20, 2012, 3:11 p.m. UTC | #3
2012/12/20 Theodore Ts'o <tytso@mit.edu>:
> On Thu, Dec 13, 2012 at 11:32:22PM +0800, Forrest Liu wrote:
>> When depth of extent tree is greater than 1, logical start value
>> of interior node didn't updated correctly in ext4_ext_rm_idx.
>>
>> Signed-off-by: Forrest Liu <forrestl@synology.com>
>
> Applied.
>
> BTW, your reproduction case also results in a file system which isn't
> noticed as being broken by e2fsck.  Eric's patch "e2fsck: Fix
> incorrect interior node logical start values" fixes e2fsck so it
> handles this.  Unfortunately applying his patch seems to uncover a bug
> in e2fsck when clearing a bad extent node (f_extent_bad_node) which we
> need to fix so the regression test suite is passing.
>
>                                             - Ted

Hi Ted,
   I will help to find out the problem in e2fsck.

Thanks,
Forrest
--
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
Theodore Ts'o Dec. 20, 2012, 11:42 p.m. UTC | #4
On Thu, Dec 20, 2012 at 11:11:28PM +0800, Forrest Liu wrote:
> Hi Ted,
>    I will help to find out the problem in e2fsck.

Thanks for the offer, but I think I've found the problem and have the
following set of patches (versus the maint branch) to fix the problem.

						- 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/extents.c b/fs/ext4/extents.c
index d3dd618..496952e 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2190,13 +2190,14 @@  errout:
  * removes index from the index block.
  */
 static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
-			struct ext4_ext_path *path)
+			struct ext4_ext_path *path, int depth)
 {
 	int err;
 	ext4_fsblk_t leaf;

 	/* free index block */
-	path--;
+	depth--;
+	path = path + depth;
 	leaf = ext4_idx_pblock(path->p_idx);
 	if (unlikely(path->p_hdr->eh_entries == 0)) {
 		EXT4_ERROR_INODE(inode, "path->p_hdr->eh_entries == 0");
@@ -2221,6 +2222,19 @@  static int ext4_ext_rm_idx(handle_t *handle,
struct inode *inode,

 	ext4_free_blocks(handle, inode, NULL, leaf, 1,
 			 EXT4_FREE_BLOCKS_METADATA | EXT4_FREE_BLOCKS_FORGET);
+
+	while (--depth >= 0) {
+		if (path->p_idx != EXT_FIRST_INDEX(path->p_hdr))
+			break;
+		path--;
+		err = ext4_ext_get_access(handle, inode, path);
+		if (err)
+			break;
+		path->p_idx->ei_block = (path+1)->p_idx->ei_block;
+		err = ext4_ext_dirty(handle, inode, path);
+		if (err)
+			break;
+	}
 	return err;
 }