[3/7] ext2fs: Update allocation info earlier in ext2fs_mkdir() and ext2fs_symlink()
diff mbox series

Message ID 20200213101602.29096-4-jack@suse.cz
State New
Headers show
Series
  • e2fsprogs: Better handling of indexed directories
Related show

Commit Message

Jan Kara Feb. 13, 2020, 10:15 a.m. UTC
Currently, ext2fs_mkdir() and ext2fs_symlink() update allocation bitmaps
and other information only close to the end of the function, in
particular after calling to ext2fs_link(). When ext2fs_link() will
support indexed directories, it will also need to allocate blocks and
that would cause filesystem corruption in case allocation info isn't
properly updated. So make sure ext2fs_mkdir() and ext2fs_symlink()
update allocation info before calling into ext2fs_link().

Signed-off-by: Jan Kara <jack@suse.cz>
---
 lib/ext2fs/mkdir.c   | 14 +++++++-------
 lib/ext2fs/symlink.c | 14 +++++++-------
 2 files changed, 14 insertions(+), 14 deletions(-)

Comments

Andreas Dilger Feb. 14, 2020, 7:37 p.m. UTC | #1
On Feb 13, 2020, at 3:15 AM, Jan Kara <jack@suse.cz> wrote:
> 
> Currently, ext2fs_mkdir() and ext2fs_symlink() update allocation bitmaps
> and other information only close to the end of the function, in
> particular after calling to ext2fs_link(). When ext2fs_link() will
> support indexed directories, it will also need to allocate blocks and
> that would cause filesystem corruption in case allocation info isn't
> properly updated. So make sure ext2fs_mkdir() and ext2fs_symlink()
> update allocation info before calling into ext2fs_link().
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

I was wondering if this was done at the end of the function to avoid the
need to undo it if there was an error in the middle of the operation?
I suppose the worst that would happen in that case is an extra bit set
in the block bitmap until the next e2fsck, which is a relatively safe
side-effect...  I'm not sure whether e2fsck would abort anyway in the
case either of these functions return an error?

In any case, this is better than what is there currently.

Reviewed-by: Andreas Dilger <adilger@dilger.ca>


> ---
> lib/ext2fs/mkdir.c   | 14 +++++++-------
> lib/ext2fs/symlink.c | 14 +++++++-------
> 2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/ext2fs/mkdir.c b/lib/ext2fs/mkdir.c
> index 2a63aad16715..947003ebf309 100644
> --- a/lib/ext2fs/mkdir.c
> +++ b/lib/ext2fs/mkdir.c
> @@ -143,6 +143,13 @@ errcode_t ext2fs_mkdir(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t inum,
> 		}
> 	}
> 
> +	/*
> +	 * Update accounting....
> +	 */
> +	if (!inline_data)
> +		ext2fs_block_alloc_stats2(fs, blk, +1);
> +	ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
> +
> 	/*
> 	 * Link the directory into the filesystem hierarchy
> 	 */
> @@ -175,13 +182,6 @@ errcode_t ext2fs_mkdir(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t inum,
> 			goto cleanup;
> 	}
> 
> -	/*
> -	 * Update accounting....
> -	 */
> -	if (!inline_data)
> -		ext2fs_block_alloc_stats2(fs, blk, +1);
> -	ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
> -
> cleanup:
> 	if (block)
> 		ext2fs_free_mem(&block);
> diff --git a/lib/ext2fs/symlink.c b/lib/ext2fs/symlink.c
> index 7f78c5f75549..3e07a539daf3 100644
> --- a/lib/ext2fs/symlink.c
> +++ b/lib/ext2fs/symlink.c
> @@ -162,6 +162,13 @@ need_block:
> 			goto cleanup;
> 	}
> 
> +	/*
> +	 * Update accounting....
> +	 */
> +	if (!fastlink && !inlinelink)
> +		ext2fs_block_alloc_stats2(fs, blk, +1);
> +	ext2fs_inode_alloc_stats2(fs, ino, +1, 0);
> +
> 	/*
> 	 * Link the symlink into the filesystem hierarchy
> 	 */
> @@ -179,13 +186,6 @@ need_block:
> 			goto cleanup;
> 	}
> 
> -	/*
> -	 * Update accounting....
> -	 */
> -	if (!fastlink && !inlinelink)
> -		ext2fs_block_alloc_stats2(fs, blk, +1);
> -	ext2fs_inode_alloc_stats2(fs, ino, +1, 0);
> -
> cleanup:
> 	if (block_buf)
> 		ext2fs_free_mem(&block_buf);
> --
> 2.16.4
> 


Cheers, Andreas

Patch
diff mbox series

diff --git a/lib/ext2fs/mkdir.c b/lib/ext2fs/mkdir.c
index 2a63aad16715..947003ebf309 100644
--- a/lib/ext2fs/mkdir.c
+++ b/lib/ext2fs/mkdir.c
@@ -143,6 +143,13 @@  errcode_t ext2fs_mkdir(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t inum,
 		}
 	}
 
+	/*
+	 * Update accounting....
+	 */
+	if (!inline_data)
+		ext2fs_block_alloc_stats2(fs, blk, +1);
+	ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
+
 	/*
 	 * Link the directory into the filesystem hierarchy
 	 */
@@ -175,13 +182,6 @@  errcode_t ext2fs_mkdir(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t inum,
 			goto cleanup;
 	}
 
-	/*
-	 * Update accounting....
-	 */
-	if (!inline_data)
-		ext2fs_block_alloc_stats2(fs, blk, +1);
-	ext2fs_inode_alloc_stats2(fs, ino, +1, 1);
-
 cleanup:
 	if (block)
 		ext2fs_free_mem(&block);
diff --git a/lib/ext2fs/symlink.c b/lib/ext2fs/symlink.c
index 7f78c5f75549..3e07a539daf3 100644
--- a/lib/ext2fs/symlink.c
+++ b/lib/ext2fs/symlink.c
@@ -162,6 +162,13 @@  need_block:
 			goto cleanup;
 	}
 
+	/*
+	 * Update accounting....
+	 */
+	if (!fastlink && !inlinelink)
+		ext2fs_block_alloc_stats2(fs, blk, +1);
+	ext2fs_inode_alloc_stats2(fs, ino, +1, 0);
+
 	/*
 	 * Link the symlink into the filesystem hierarchy
 	 */
@@ -179,13 +186,6 @@  need_block:
 			goto cleanup;
 	}
 
-	/*
-	 * Update accounting....
-	 */
-	if (!fastlink && !inlinelink)
-		ext2fs_block_alloc_stats2(fs, blk, +1);
-	ext2fs_inode_alloc_stats2(fs, ino, +1, 0);
-
 cleanup:
 	if (block_buf)
 		ext2fs_free_mem(&block_buf);