2013-01-03 14:13:35

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH 1/4] resize2fs: fix 32-bit overflow issue which can corrupt 64-bit file systems

Fix a 32-bit overflow bug caused by a missing blk64_t cast which can
cause the block bitmap to get corrupted when doing an off-line resize
of a 64-bit file system.

This problem can be reproduced as follows:

rm -f foo.img; touch foo.img
truncate -s 8T foo.img
mke2fs -F -t ext4 -O 64bit foo.img
e2fsck -f foo.img
truncate -s 21T foo.img
resize2fs foo.img
e2fsck -fy foo.img

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
resize/resize2fs.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index 092cfbd..0407e41 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -197,8 +197,7 @@ static void fix_uninit_block_bitmaps(ext2_filsys fs)
if (!(ext2fs_bg_flags_test(fs, g, EXT2_BG_BLOCK_UNINIT)))
continue;

- blk = (g * fs->super->s_blocks_per_group) +
- fs->super->s_first_data_block;
+ blk = ext2fs_group_first_block2(fs, g);

ext2fs_super_and_bgd_loc2(fs, g, &super_blk,
&old_desc_blk, &new_desc_blk, 0);
@@ -846,8 +845,7 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
* The block bitmap is uninitialized, so skip
* to the next block group.
*/
- blk = ((g+1) * fs->super->s_blocks_per_group) +
- fs->super->s_first_data_block - 1;
+ blk = ext2fs_group_first_block2(fs, g+1) - 1;
continue;
}
if (ext2fs_test_block_bitmap2(old_fs->block_map, blk) &&
--
1.7.12.rc0.22.gcdd159b



2013-01-03 14:13:30

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH 2/4] resize2fs: fix 32-bit overflow when calculating the number of free blocks

This caused the free blocks count in the superblock to be incorrect
after resizing a 64-bit file system if the number of free blocks
overflowed a 32-bit value.

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
resize/resize2fs.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index 0407e41..d9501f9 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -1795,7 +1795,8 @@ static errcode_t ext2fs_calculate_summary_stats(ext2_filsys fs)
ext2_ino_t ino;
unsigned int group = 0;
unsigned int count = 0;
- int total_free = 0;
+ blk64_t total_blocks_free = 0;
+ int total_inodes_free = 0;
int group_free = 0;
int uninit = 0;
blk64_t super_blk, old_desc_blk, new_desc_blk;
@@ -1827,7 +1828,7 @@ static errcode_t ext2fs_calculate_summary_stats(ext2_filsys fs)
+ fs->inode_blocks_per_group))))) ||
(!ext2fs_fast_test_block_bitmap2(fs->block_map, blk))) {
group_free++;
- total_free++;
+ total_blocks_free++;
}
count++;
if ((count == fs->super->s_blocks_per_group) ||
@@ -1851,13 +1852,12 @@ static errcode_t ext2fs_calculate_summary_stats(ext2_filsys fs)
fs->super->s_reserved_gdt_blocks;
}
}
- ext2fs_free_blocks_count_set(fs->super, total_free);
+ ext2fs_free_blocks_count_set(fs->super, total_blocks_free);

/*
* Next, calculate the inode statistics
*/
group_free = 0;
- total_free = 0;
count = 0;
group = 0;

@@ -1867,7 +1867,7 @@ static errcode_t ext2fs_calculate_summary_stats(ext2_filsys fs)
if (uninit ||
!ext2fs_fast_test_inode_bitmap2(fs->inode_map, ino)) {
group_free++;
- total_free++;
+ total_inodes_free++;
}
count++;
if ((count == fs->super->s_inodes_per_group) ||
@@ -1882,7 +1882,7 @@ static errcode_t ext2fs_calculate_summary_stats(ext2_filsys fs)
uninit = ext2fs_bg_flags_test(fs, group, EXT2_BG_INODE_UNINIT);
}
}
- fs->super->s_free_inodes_count = total_free;
+ fs->super->s_free_inodes_count = total_inodes_free;
ext2fs_mark_super_dirty(fs);
return 0;
}
--
1.7.12.rc0.22.gcdd159b


2013-01-03 14:13:30

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH 4/4] resize2fs: use [un]mark_block_range bitmap functions to reduce CPU usage

Use ext2fs_[un]mark_block_range2() functions to reduce the CPU
overhead of resizing large file systems by 45%, primarily by
reducing the time spent in fix_uninit_block_bitmaps().

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
lib/ext2fs/alloc_sb.c | 9 ++++---
resize/resize2fs.c | 67 ++++++++++++++++++---------------------------------
2 files changed, 29 insertions(+), 47 deletions(-)

diff --git a/lib/ext2fs/alloc_sb.c b/lib/ext2fs/alloc_sb.c
index 0d1c000..868d724 100644
--- a/lib/ext2fs/alloc_sb.c
+++ b/lib/ext2fs/alloc_sb.c
@@ -67,10 +67,11 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs,
if (old_desc_blk) {
if (fs->super->s_reserved_gdt_blocks && fs->block_map == bmap)
ext2fs_bg_flags_clear(fs, group, EXT2_BG_BLOCK_UNINIT);
- for (j=0; j < old_desc_blocks; j++)
- if (old_desc_blk + j < ext2fs_blocks_count(fs->super))
- ext2fs_mark_block_bitmap2(bmap,
- old_desc_blk + j);
+ num_blocks = old_desc_blocks;
+ if (old_desc_blk + num_blocks >= ext2fs_blocks_count(fs->super))
+ num_blocks = ext2fs_blocks_count(fs->super) -
+ old_desc_blk;
+ ext2fs_mark_block_bitmap_range2(bmap, old_desc_blk, num_blocks);
}
if (new_desc_blk)
ext2fs_mark_block_bitmap2(bmap, new_desc_blk);
diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index f6a52e5..ac965ee 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -215,9 +215,9 @@ errout:
*/
static void fix_uninit_block_bitmaps(ext2_filsys fs)
{
- blk64_t i, blk, super_blk, old_desc_blk, new_desc_blk;
- int old_desc_blocks;
+ blk64_t blk, lblk;
dgrp_t g;
+ int i;

if (!(EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
EXT4_FEATURE_RO_COMPAT_GDT_CSUM)))
@@ -228,33 +228,19 @@ static void fix_uninit_block_bitmaps(ext2_filsys fs)
continue;

blk = ext2fs_group_first_block2(fs, g);
-
- ext2fs_super_and_bgd_loc2(fs, g, &super_blk,
- &old_desc_blk, &new_desc_blk, 0);
-
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks +
- fs->super->s_reserved_gdt_blocks;
-
- for (i=0; i < fs->super->s_blocks_per_group; i++, blk++) {
- if (blk >= ext2fs_blocks_count(fs->super))
- break;
- if ((blk == super_blk) ||
- (old_desc_blk && old_desc_blocks &&
- (blk >= old_desc_blk) &&
- (blk < old_desc_blk + old_desc_blocks)) ||
- (new_desc_blk && (blk == new_desc_blk)) ||
- (blk == ext2fs_block_bitmap_loc(fs, g)) ||
- (blk == ext2fs_inode_bitmap_loc(fs, g)) ||
- (blk >= ext2fs_inode_table_loc(fs, g) &&
- (blk < ext2fs_inode_table_loc(fs, g)
- + fs->inode_blocks_per_group)))
- ext2fs_fast_mark_block_bitmap2(fs->block_map, blk);
- else
- ext2fs_fast_unmark_block_bitmap2(fs->block_map, blk);
- }
+ lblk = ext2fs_group_last_block2(fs, g);
+ ext2fs_unmark_block_bitmap_range2(fs->block_map, blk,
+ lblk - blk + 1);
+
+ ext2fs_reserve_super_and_bgd(fs, g, fs->block_map);
+ ext2fs_mark_block_bitmap2(fs->block_map,
+ ext2fs_block_bitmap_loc(fs, g));
+ ext2fs_mark_block_bitmap2(fs->block_map,
+ ext2fs_inode_bitmap_loc(fs, g));
+ for (i = 0, blk = ext2fs_inode_table_loc(fs, g);
+ i < (unsigned int) fs->inode_blocks_per_group;
+ i++, blk++)
+ ext2fs_mark_block_bitmap2(fs->block_map, blk);
}
}

@@ -722,8 +708,6 @@ errout:
static errcode_t mark_table_blocks(ext2_filsys fs,
ext2fs_block_bitmap bmap)
{
- blk64_t b;
- unsigned int j;
dgrp_t i;

for (i = 0; i < fs->group_desc_count; i++) {
@@ -732,10 +716,9 @@ static errcode_t mark_table_blocks(ext2_filsys fs,
/*
* Mark the blocks used for the inode table
*/
- for (j = 0, b = ext2fs_inode_table_loc(fs, i);
- j < (unsigned int) fs->inode_blocks_per_group;
- j++, b++)
- ext2fs_mark_block_bitmap2(bmap, b);
+ ext2fs_mark_block_bitmap_range2(bmap,
+ ext2fs_inode_table_loc(fs, i),
+ fs->inode_blocks_per_group);

/*
* Mark block used for the block bitmap
@@ -982,15 +965,13 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
ext2fs_inode_bitmap_loc(old_fs, i));

if (ext2fs_inode_table_loc(fs, i))
- for (blk = ext2fs_inode_table_loc(fs, i), j=0;
- j < fs->inode_blocks_per_group ; j++, blk++)
- ext2fs_mark_block_bitmap2(rfs->reserve_blocks,
- blk);
+ ext2fs_mark_block_bitmap_range2(rfs->reserve_blocks,
+ ext2fs_inode_table_loc(fs, i),
+ fs->inode_blocks_per_group);
else if (flex_bg && i < old_fs->group_desc_count)
- for (blk = ext2fs_inode_table_loc(old_fs, i), j=0;
- j < old_fs->inode_blocks_per_group ; j++, blk++)
- ext2fs_mark_block_bitmap2(rfs->reserve_blocks,
- blk);
+ ext2fs_mark_block_bitmap_range2(rfs->reserve_blocks,
+ ext2fs_inode_table_loc(old_fs, i),
+ old_fs->inode_blocks_per_group);

group_blk += rfs->new_fs->super->s_blocks_per_group;
}
--
1.7.12.rc0.22.gcdd159b


2013-01-03 14:13:35

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH 3/4] resize2fs: add resource tracking as a debug option

Add a new debug flag which prints how much time is consumed by the
various parts of resize2fs's processing.

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
resize/Makefile.in | 27 +++++++---
resize/resize2fs.8.in | 2 +
resize/resize2fs.c | 50 +++++++++++++++----
resize/resize2fs.h | 22 +++++++++
resize/resource_track.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 213 insertions(+), 16 deletions(-)
create mode 100644 resize/resource_track.c

diff --git a/resize/Makefile.in b/resize/Makefile.in
index b2739ac..a06b642 100644
--- a/resize/Makefile.in
+++ b/resize/Makefile.in
@@ -16,7 +16,8 @@ PROGS= resize2fs
TEST_PROGS= test_extent
MANPAGES= resize2fs.8

-RESIZE_OBJS= extent.o resize2fs.o main.o online.o sim_progress.o
+RESIZE_OBJS= extent.o resize2fs.o main.o online.o resource_track.o \
+ sim_progress.o

TEST_EXTENT_OBJS= extent.o test_extent.o

@@ -24,6 +25,7 @@ SRCS= $(srcdir)/extent.c \
$(srcdir)/resize2fs.c \
$(srcdir)/main.c \
$(srcdir)/online.c \
+ $(srcdir)/resource_track.c \
$(srcdir)/sim_progress.c

LIBS= $(LIBE2P) $(LIBEXT2FS) $(LIBCOM_ERR) $(LIBINTL)
@@ -144,35 +146,48 @@ source_tar_file: $(top_srcdir)/.exclude-file
# Makefile dependencies follow. This must be the last section in
# the Makefile.in file
#
-extent.o: $(srcdir)/extent.c $(srcdir)/resize2fs.h \
+extent.o: $(srcdir)/extent.c $(top_builddir)/lib/config.h \
+ $(top_builddir)/lib/dirpaths.h $(srcdir)/resize2fs.h \
$(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h \
$(top_srcdir)/lib/ext2fs/ext2fs.h $(top_srcdir)/lib/ext2fs/ext3_extents.h \
$(top_srcdir)/lib/et/com_err.h $(top_srcdir)/lib/ext2fs/ext2_io.h \
$(top_builddir)/lib/ext2fs/ext2_err.h \
$(top_srcdir)/lib/ext2fs/ext2_ext_attr.h $(top_srcdir)/lib/ext2fs/bitops.h \
$(top_srcdir)/lib/e2p/e2p.h
-resize2fs.o: $(srcdir)/resize2fs.c $(srcdir)/resize2fs.h \
+resize2fs.o: $(srcdir)/resize2fs.c $(top_builddir)/lib/config.h \
+ $(top_builddir)/lib/dirpaths.h $(srcdir)/resize2fs.h \
$(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h \
$(top_srcdir)/lib/ext2fs/ext2fs.h $(top_srcdir)/lib/ext2fs/ext3_extents.h \
$(top_srcdir)/lib/et/com_err.h $(top_srcdir)/lib/ext2fs/ext2_io.h \
$(top_builddir)/lib/ext2fs/ext2_err.h \
$(top_srcdir)/lib/ext2fs/ext2_ext_attr.h $(top_srcdir)/lib/ext2fs/bitops.h \
$(top_srcdir)/lib/e2p/e2p.h
-main.o: $(srcdir)/main.c $(top_srcdir)/lib/e2p/e2p.h \
+main.o: $(srcdir)/main.c $(top_builddir)/lib/config.h \
+ $(top_builddir)/lib/dirpaths.h $(top_srcdir)/lib/e2p/e2p.h \
$(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h \
$(srcdir)/resize2fs.h $(top_srcdir)/lib/ext2fs/ext2fs.h \
$(top_srcdir)/lib/ext2fs/ext3_extents.h $(top_srcdir)/lib/et/com_err.h \
$(top_srcdir)/lib/ext2fs/ext2_io.h $(top_builddir)/lib/ext2fs/ext2_err.h \
$(top_srcdir)/lib/ext2fs/ext2_ext_attr.h $(top_srcdir)/lib/ext2fs/bitops.h \
$(top_srcdir)/version.h
-online.o: $(srcdir)/online.c $(srcdir)/resize2fs.h \
+online.o: $(srcdir)/online.c $(top_builddir)/lib/config.h \
+ $(top_builddir)/lib/dirpaths.h $(srcdir)/resize2fs.h \
$(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h \
$(top_srcdir)/lib/ext2fs/ext2fs.h $(top_srcdir)/lib/ext2fs/ext3_extents.h \
$(top_srcdir)/lib/et/com_err.h $(top_srcdir)/lib/ext2fs/ext2_io.h \
$(top_builddir)/lib/ext2fs/ext2_err.h \
$(top_srcdir)/lib/ext2fs/ext2_ext_attr.h $(top_srcdir)/lib/ext2fs/bitops.h \
$(top_srcdir)/lib/e2p/e2p.h
-sim_progress.o: $(srcdir)/sim_progress.c $(srcdir)/resize2fs.h \
+resource_track.o: $(srcdir)/resource_track.c $(top_builddir)/lib/config.h \
+ $(top_builddir)/lib/dirpaths.h $(srcdir)/resize2fs.h \
+ $(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h \
+ $(top_srcdir)/lib/ext2fs/ext2fs.h $(top_srcdir)/lib/ext2fs/ext3_extents.h \
+ $(top_srcdir)/lib/et/com_err.h $(top_srcdir)/lib/ext2fs/ext2_io.h \
+ $(top_builddir)/lib/ext2fs/ext2_err.h \
+ $(top_srcdir)/lib/ext2fs/ext2_ext_attr.h $(top_srcdir)/lib/ext2fs/bitops.h \
+ $(top_srcdir)/lib/e2p/e2p.h
+sim_progress.o: $(srcdir)/sim_progress.c $(top_builddir)/lib/config.h \
+ $(top_builddir)/lib/dirpaths.h $(srcdir)/resize2fs.h \
$(top_srcdir)/lib/ext2fs/ext2_fs.h $(top_builddir)/lib/ext2fs/ext2_types.h \
$(top_srcdir)/lib/ext2fs/ext2fs.h $(top_srcdir)/lib/ext2fs/ext3_extents.h \
$(top_srcdir)/lib/et/com_err.h $(top_srcdir)/lib/ext2fs/ext2_io.h \
diff --git a/resize/resize2fs.8.in b/resize/resize2fs.8.in
index 9ff6e0a..735fc91 100644
--- a/resize/resize2fs.8.in
+++ b/resize/resize2fs.8.in
@@ -99,6 +99,8 @@ from the following list:
4 \-\ Debug inode relocations
.br
8 \-\ Debug moving the inode table
+.br
+ 16 \-\ Print timing information
.TP
.B \-f
Forces resize2fs to proceed with the filesystem resize operation, overriding
diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index d9501f9..f6a52e5 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -74,14 +74,7 @@ errcode_t resize_fs(ext2_filsys fs, blk64_t *new_size, int flags,
{
ext2_resize_t rfs;
errcode_t retval;
-
- retval = ext2fs_read_bitmaps(fs);
- if (retval)
- return retval;
-
- fs->super->s_state |= EXT2_ERROR_FS;
- ext2fs_mark_super_dirty(fs);
- ext2fs_flush(fs);
+ struct resource_track rtrack, overall_track;

/*
* Create the data structure
@@ -89,32 +82,53 @@ errcode_t resize_fs(ext2_filsys fs, blk64_t *new_size, int flags,
retval = ext2fs_get_mem(sizeof(struct ext2_resize_struct), &rfs);
if (retval)
return retval;
- memset(rfs, 0, sizeof(struct ext2_resize_struct));

- fix_uninit_block_bitmaps(fs);
+ memset(rfs, 0, sizeof(struct ext2_resize_struct));
fs->priv_data = rfs;
rfs->old_fs = fs;
rfs->flags = flags;
rfs->itable_buf = 0;
rfs->progress = progress;
+
+ init_resource_track(&overall_track, "overall resize2fs", fs->io);
+ init_resource_track(&rtrack, "read_bitmaps", fs->io);
+ retval = ext2fs_read_bitmaps(fs);
+ if (retval)
+ goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);
+
+ fs->super->s_state |= EXT2_ERROR_FS;
+ ext2fs_mark_super_dirty(fs);
+ ext2fs_flush(fs);
+
+ init_resource_track(&rtrack, "fix_uninit_block_bitmaps 1", fs->io);
+ fix_uninit_block_bitmaps(fs);
+ print_resource_track(rfs, &rtrack, fs->io);
retval = ext2fs_dup_handle(fs, &rfs->new_fs);
if (retval)
goto errout;

+ init_resource_track(&rtrack, "adjust_superblock", fs->io);
retval = adjust_superblock(rfs, *new_size);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

+
+ init_resource_track(&rtrack, "fix_uninit_block_bitmaps 2", fs->io);
fix_uninit_block_bitmaps(rfs->new_fs);
+ print_resource_track(rfs, &rtrack, fs->io);
/* Clear the block bitmap uninit flag for the last block group */
ext2fs_bg_flags_clear(rfs->new_fs, rfs->new_fs->group_desc_count - 1,
EXT2_BG_BLOCK_UNINIT);

*new_size = ext2fs_blocks_count(rfs->new_fs->super);

+ init_resource_track(&rtrack, "blocks_to_move", fs->io);
retval = blocks_to_move(rfs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

#ifdef RESIZE2FS_DEBUG
if (rfs->flags & RESIZE_DEBUG_BMOVE)
@@ -124,36 +138,52 @@ errcode_t resize_fs(ext2_filsys fs, blk64_t *new_size, int flags,
rfs->needed_blocks);
#endif

+ init_resource_track(&rtrack, "block_mover", fs->io);
retval = block_mover(rfs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

+ init_resource_track(&rtrack, "inode_scan_and_fix", fs->io);
retval = inode_scan_and_fix(rfs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

+ init_resource_track(&rtrack, "inode_ref_fix", fs->io);
retval = inode_ref_fix(rfs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

+ init_resource_track(&rtrack, "move_itables", fs->io);
retval = move_itables(rfs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

+ init_resource_track(&rtrack, "calculate_summary_stats", fs->io);
retval = ext2fs_calculate_summary_stats(rfs->new_fs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

+ init_resource_track(&rtrack, "fix_resize_inode", fs->io);
retval = fix_resize_inode(rfs->new_fs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

+ init_resource_track(&rtrack, "fix_sb_journal_backup", fs->io);
retval = fix_sb_journal_backup(rfs->new_fs);
if (retval)
goto errout;
+ print_resource_track(rfs, &rtrack, fs->io);

rfs->new_fs->super->s_state &= ~EXT2_ERROR_FS;
rfs->new_fs->flags &= ~EXT2_FLAG_MASTER_SB_ONLY;
+
+ print_resource_track(rfs, &overall_track, fs->io);
retval = ext2fs_close(rfs->new_fs);
if (retval)
goto errout;
diff --git a/resize/resize2fs.h b/resize/resize2fs.h
index 2184759..d425491 100644
--- a/resize/resize2fs.h
+++ b/resize/resize2fs.h
@@ -76,11 +76,26 @@ typedef struct ext2_sim_progress *ext2_sim_progmeter;
#define RESIZE_DEBUG_BMOVE 0x0002
#define RESIZE_DEBUG_INODEMAP 0x0004
#define RESIZE_DEBUG_ITABLEMOVE 0x0008
+#define RESIZE_DEBUG_RTRACK 0x0010

#define RESIZE_PERCENT_COMPLETE 0x0100
#define RESIZE_VERBOSE 0x0200

/*
+ * This structure is used for keeping track of how much resources have
+ * been used for a particular resize2fs pass.
+ */
+struct resource_track {
+ const char *desc;
+ struct timeval time_start;
+ struct timeval user_start;
+ struct timeval system_start;
+ void *brk_start;
+ unsigned long long bytes_read;
+ unsigned long long bytes_written;
+};
+
+/*
* The core state structure for the ext2 resizer
*/
typedef struct ext2_resize_struct *ext2_resize_t;
@@ -148,6 +163,13 @@ extern errcode_t ext2fs_iterate_extent(ext2_extent extent, __u64 *old_loc,
extern errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
blk64_t *new_size, int flags);

+/* resource_track.c */
+extern void init_resource_track(struct resource_track *track, const char *desc,
+ io_channel channel);
+extern void print_resource_track(ext2_resize_t rfs,
+ struct resource_track *track,
+ io_channel channel);
+
/* sim_progress.c */
extern errcode_t ext2fs_progress_init(ext2_sim_progmeter *ret_prog,
const char *label,
diff --git a/resize/resource_track.c b/resize/resource_track.c
new file mode 100644
index 0000000..f0efe11
--- /dev/null
+++ b/resize/resource_track.c
@@ -0,0 +1,128 @@
+/*
+ * resource_track.c --- resource tracking
+ *
+ * Copyright (C) 2013 by Theodore Ts'o
+ *
+ * %Begin-Header%
+ * This file may be redistributed under the terms of the GNU Public
+ * License.
+ * %End-Header%
+ */
+
+
+#include "config.h"
+#include "resize2fs.h"
+#include <time.h>
+#ifdef HAVE_MALLOC_H
+#include <malloc.h>
+#endif
+#include <sys/resource.h>
+
+void init_resource_track(struct resource_track *track, const char *desc,
+ io_channel channel)
+{
+#ifdef HAVE_GETRUSAGE
+ struct rusage r;
+#endif
+ io_stats io_start = 0;
+
+ track->desc = desc;
+ track->brk_start = sbrk(0);
+ gettimeofday(&track->time_start, 0);
+#ifdef HAVE_GETRUSAGE
+#ifdef sun
+ memset(&r, 0, sizeof(struct rusage));
+#endif
+ getrusage(RUSAGE_SELF, &r);
+ track->user_start = r.ru_utime;
+ track->system_start = r.ru_stime;
+#else
+ track->user_start.tv_sec = track->user_start.tv_usec = 0;
+ track->system_start.tv_sec = track->system_start.tv_usec = 0;
+#endif
+ track->bytes_read = 0;
+ track->bytes_written = 0;
+ if (channel && channel->manager && channel->manager->get_stats)
+ channel->manager->get_stats(channel, &io_start);
+ if (io_start) {
+ track->bytes_read = io_start->bytes_read;
+ track->bytes_written = io_start->bytes_written;
+ }
+}
+
+static float timeval_subtract(struct timeval *tv1,
+ struct timeval *tv2)
+{
+ return ((tv1->tv_sec - tv2->tv_sec) +
+ ((float) (tv1->tv_usec - tv2->tv_usec)) / 1000000);
+}
+
+void print_resource_track(ext2_resize_t rfs, struct resource_track *track,
+ io_channel channel)
+{
+#ifdef HAVE_GETRUSAGE
+ struct rusage r;
+#endif
+#ifdef HAVE_MALLINFO
+ struct mallinfo malloc_info;
+#endif
+ struct timeval time_end;
+
+ if ((rfs->flags & RESIZE_DEBUG_RTRACK) == 0)
+ return;
+
+ gettimeofday(&time_end, 0);
+
+ if (track->desc)
+ printf("%s: ", track->desc);
+
+#ifdef HAVE_MALLINFO
+#define kbytes(x) (((unsigned long)(x) + 1023) / 1024)
+
+ malloc_info = mallinfo();
+ printf("Memory used: %luk/%luk (%luk/%luk), ",
+ kbytes(malloc_info.arena), kbytes(malloc_info.hblkhd),
+ kbytes(malloc_info.uordblks), kbytes(malloc_info.fordblks));
+#else
+ printf("Memory used: %lu, ",
+ (unsigned long) (((char *) sbrk(0)) -
+ ((char *) track->brk_start)));
+#endif
+#ifdef HAVE_GETRUSAGE
+ getrusage(RUSAGE_SELF, &r);
+
+ printf("time: %5.2f/%5.2f/%5.2f\n",
+ timeval_subtract(&time_end, &track->time_start),
+ timeval_subtract(&r.ru_utime, &track->user_start),
+ timeval_subtract(&r.ru_stime, &track->system_start));
+#else
+ printf("elapsed time: %6.3f\n",
+ timeval_subtract(&time_end, &track->time_start));
+#endif
+#define mbytes(x) (((x) + 1048575) / 1048576)
+ if (channel && channel->manager && channel->manager->get_stats) {
+ io_stats delta = 0;
+ unsigned long long bytes_read = 0;
+ unsigned long long bytes_written = 0;
+
+ channel->manager->get_stats(channel, &delta);
+ if (delta) {
+ bytes_read = delta->bytes_read - track->bytes_read;
+ bytes_written = delta->bytes_written -
+ track->bytes_written;
+ if (bytes_read == 0 && bytes_written == 0)
+ goto skip_io;
+ if (track->desc)
+ printf("%s: ", track->desc);
+ printf("I/O read: %lluMB, write: %lluMB, "
+ "rate: %.2fMB/s\n",
+ mbytes(bytes_read),
+ mbytes(bytes_written),
+ (double)mbytes(bytes_read + bytes_written) /
+ timeval_subtract(&time_end, &track->time_start));
+ }
+ }
+skip_io:
+ fflush(stdout);
+}
+
--
1.7.12.rc0.22.gcdd159b


2013-01-03 15:56:51

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH 1/4] resize2fs: fix 32-bit overflow issue which can corrupt 64-bit file systems

On 1/3/13 8:13 AM, Theodore Ts'o wrote:
> Fix a 32-bit overflow bug caused by a missing blk64_t cast which can
> cause the block bitmap to get corrupted when doing an off-line resize
> of a 64-bit file system.

Yikes - seems like there are quite a few places where we need to
audit this kind of thing

4 resize/online.c <global> 171 size = fs->group_desc_count * sb->s_blocks_per_group +

size is a blk_t / __u32

a e2fsck/super.c check_resize_inode 421 expect = pblk + (j * fs->super->s_blocks_per_group);

j is dgrp_t / __u32

f e2fsck/super.c check_backup_super_block 931 (g * fs->super->s_blocks_per_group);

g is dgrp_t / __u32

1 lib/ext2fs/alloc.c check_block_uninit 43 blk = (group * fs->super->s_blocks_per_group) +

group is dgrp_t / __u32

e lib/ext2fs/extent.c extent_node_split 947 goal_blk = (group * handle->fs->super->s_blocks_per_group) +

group is dgrp_t ...


1 lib/ext2fs/mkjournal.c write_journal_inode 361 es.goal = (fs->super->s_blocks_per_group * group) +
8 resize/resize2fs.c blocks_to_move 828 blk = ((g+1) * fs->super->s_blocks_per_group) +

... etc. Some of those might not matter (we might not get into that resize code) but this look like a lot of potential for trouble.

-Eric



> This problem can be reproduced as follows:
>
> rm -f foo.img; touch foo.img
> truncate -s 8T foo.img
> mke2fs -F -t ext4 -O 64bit foo.img
> e2fsck -f foo.img
> truncate -s 21T foo.img
> resize2fs foo.img
> e2fsck -fy foo.img
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>
> ---
> resize/resize2fs.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/resize/resize2fs.c b/resize/resize2fs.c
> index 092cfbd..0407e41 100644
> --- a/resize/resize2fs.c
> +++ b/resize/resize2fs.c
> @@ -197,8 +197,7 @@ static void fix_uninit_block_bitmaps(ext2_filsys fs)
> if (!(ext2fs_bg_flags_test(fs, g, EXT2_BG_BLOCK_UNINIT)))
> continue;
>
> - blk = (g * fs->super->s_blocks_per_group) +
> - fs->super->s_first_data_block;
> + blk = ext2fs_group_first_block2(fs, g);
>
> ext2fs_super_and_bgd_loc2(fs, g, &super_blk,
> &old_desc_blk, &new_desc_blk, 0);
> @@ -846,8 +845,7 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
> * The block bitmap is uninitialized, so skip
> * to the next block group.
> */
> - blk = ((g+1) * fs->super->s_blocks_per_group) +
> - fs->super->s_first_data_block - 1;
> + blk = ext2fs_group_first_block2(fs, g+1) - 1;
> continue;
> }
> if (ext2fs_test_block_bitmap2(old_fs->block_map, blk) &&
>


2013-01-03 17:16:39

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/4] resize2fs: fix 32-bit overflow issue which can corrupt 64-bit file systems

On Thu, Jan 03, 2013 at 09:56:51AM -0600, Eric Sandeen wrote:
>
> Yikes - seems like there are quite a few places where we need to
> audit this kind of thing

Fortunately a bunch of these only apply for 32-bit resizing (i..e,
involving the resize_inode or the 32-bit resize iocl). The goal_blk
calculations just mean that we will be using a non-optimal block
number, which we should fix, but it's not catastrophic.

The check_block_uninit() function in lib/ext2fs/alloc.c could
defintely cause a problem if someone were to use the library to write
into a 64-bit file system via FUSE, e2tools, or debugfs, but it's
unlikely to cause a problem for mke2fs or e2fsck. (It could
potentially cause a problem if e2fsck needed to freshly allocate some
new blocks for e.g., a missing lost+found directory, or during pass1b
processing and it allocates for the first time into an block group
with BLOCK_UNINIT, but it's not a high probability bug.)

Regardless of how likely they are, I agree absolutely that we should
audit and fix all of these problems.

- Ted

2013-01-03 18:57:58

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH] Fix 32-bit overflow problems: dgrp_t * s_blocks_per_group

There are a number of places where we multiply a dgrp_t with
s_blocks_per_group expecting that we will get a blk64_t. This
requires a cast, or using the convenience function
ext2fs_group_first_block2().

This audit was suggested by Eric Sandeen.

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
e2fsck/super.c | 6 +++---
lib/ext2fs/alloc.c | 6 ++----
lib/ext2fs/extent.c | 3 +--
lib/ext2fs/mkjournal.c | 4 +---
resize/resize2fs.c | 10 ++++------
5 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/e2fsck/super.c b/e2fsck/super.c
index 160991d..b4b5bff 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -317,7 +317,8 @@ void check_resize_inode(e2fsck_t ctx)
struct problem_context pctx;
int i, gdt_off, ind_off;
dgrp_t j;
- blk64_t blk, pblk, expect;
+ blk64_t blk, pblk;
+ blk_t expect;
__u32 *dind_buf = 0, *ind_buf;
errcode_t retval;

@@ -914,8 +915,7 @@ int check_backup_super_block(e2fsck_t ctx)
if (!ext2fs_bg_has_super(fs, g))
continue;

- sb = fs->super->s_first_data_block +
- (g * fs->super->s_blocks_per_group);
+ sb = ext2fs_group_first_block2(fs, g);

retval = io_channel_read_blk(fs->io, sb, -SUPERBLOCK_SIZE,
buf);
diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
index 775dfcc..0c829ed 100644
--- a/lib/ext2fs/alloc.c
+++ b/lib/ext2fs/alloc.c
@@ -41,8 +41,7 @@ static void check_block_uninit(ext2_filsys fs, ext2fs_block_bitmap map,
!(ext2fs_bg_flags_test(fs, group, EXT2_BG_BLOCK_UNINIT)))
return;

- blk = (group * fs->super->s_blocks_per_group) +
- fs->super->s_first_data_block;
+ blk = ext2fs_group_first_block2(fs, group);

ext2fs_super_and_bgd_loc2(fs, group, &super_blk,
&old_desc_blk, &new_desc_blk, 0);
@@ -56,8 +55,7 @@ static void check_block_uninit(ext2_filsys fs, ext2fs_block_bitmap map,
for (i=0; i < fs->super->s_blocks_per_group; i++, blk++)
ext2fs_fast_unmark_block_bitmap2(map, blk);

- blk = (group * fs->super->s_blocks_per_group) +
- fs->super->s_first_data_block;
+ blk = ext2fs_group_first_block2(fs, group);
for (i=0; i < fs->super->s_blocks_per_group; i++, blk++) {
if ((blk == super_blk) ||
(old_desc_blk && old_desc_blocks &&
diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index 4d6af88..9cc7cba 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -934,8 +934,7 @@ errcode_t ext2fs_extent_node_split(ext2_extent_handle_t handle)

if (log_flex)
group = group & ~((1 << (log_flex)) - 1);
- goal_blk = (group * handle->fs->super->s_blocks_per_group) +
- handle->fs->super->s_first_data_block;
+ goal_blk = ext2fs_group_first_block2(handle->fs, group);
}
retval = ext2fs_alloc_block2(handle->fs, goal_blk, block_buf,
&new_node_pblk);
diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c
index c154d91..c636a97 100644
--- a/lib/ext2fs/mkjournal.c
+++ b/lib/ext2fs/mkjournal.c
@@ -358,9 +358,7 @@ static errcode_t write_journal_inode(ext2_filsys fs, ext2_ino_t journal_ino,
ext2fs_bg_free_blocks_count(fs, group))
group = i;

- es.goal = (fs->super->s_blocks_per_group * group) +
- fs->super->s_first_data_block;
-
+ es.goal = ext2fs_group_first_block2(fs, group);
retval = ext2fs_block_iterate3(fs, journal_ino, BLOCK_FLAG_APPEND,
0, mkjournal_proc, &es);
if (es.err) {
diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index ac965ee..8fdf35c 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -492,9 +492,8 @@ retry:
/*
* Initialize the new block group descriptors
*/
- group_block = fs->super->s_first_data_block +
- old_fs->group_desc_count * fs->super->s_blocks_per_group;
-
+ group_block = ext2fs_group_first_block2(fs,
+ old_fs->group_desc_count);
csum_flag = EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
EXT4_FEATURE_RO_COMPAT_GDT_CSUM);
if (access("/sys/fs/ext4/features/lazy_itable_init", F_OK) == 0)
@@ -651,9 +650,8 @@ static errcode_t adjust_superblock(ext2_resize_t rfs, blk64_t new_size)
goto errout;

memset(rfs->itable_buf, 0, fs->blocksize * fs->inode_blocks_per_group);
- group_block = fs->super->s_first_data_block +
- rfs->old_fs->group_desc_count * fs->super->s_blocks_per_group;

2013-01-04 17:54:54

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] Fix 32-bit overflow problems: dgrp_t * s_blocks_per_group

On 1/3/13 12:57 PM, Theodore Ts'o wrote:
> There are a number of places where we multiply a dgrp_t with
> s_blocks_per_group expecting that we will get a blk64_t. This
> requires a cast, or using the convenience function
> ext2fs_group_first_block2().
>
> This audit was suggested by Eric Sandeen.
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>

Thanks Ted, didn't exactly mean to assign you work ;)

> ---
> e2fsck/super.c | 6 +++---
> lib/ext2fs/alloc.c | 6 ++----
> lib/ext2fs/extent.c | 3 +--
> lib/ext2fs/mkjournal.c | 4 +---
> resize/resize2fs.c | 10 ++++------
> 5 files changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/e2fsck/super.c b/e2fsck/super.c
> index 160991d..b4b5bff 100644
> --- a/e2fsck/super.c
> +++ b/e2fsck/super.c
> @@ -317,7 +317,8 @@ void check_resize_inode(e2fsck_t ctx)
> struct problem_context pctx;
> int i, gdt_off, ind_off;
> dgrp_t j;
> - blk64_t blk, pblk, expect;
> + blk64_t blk, pblk;
> + blk_t expect;

this is ok because resize-inode-land is all < 32 bits I guess, right?
A comment might help, but *shrug*

The rest:

Reviewed-by: Eric Sandeen <[email protected]>

Wish the were a way to enforce use of the helper function, these
may well creep back in.

-Eric

> __u32 *dind_buf = 0, *ind_buf;
> errcode_t retval;
>
> @@ -914,8 +915,7 @@ int check_backup_super_block(e2fsck_t ctx)
> if (!ext2fs_bg_has_super(fs, g))
> continue;
>
> - sb = fs->super->s_first_data_block +
> - (g * fs->super->s_blocks_per_group);
> + sb = ext2fs_group_first_block2(fs, g);
>
> retval = io_channel_read_blk(fs->io, sb, -SUPERBLOCK_SIZE,
> buf);
> diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
> index 775dfcc..0c829ed 100644
> --- a/lib/ext2fs/alloc.c
> +++ b/lib/ext2fs/alloc.c
> @@ -41,8 +41,7 @@ static void check_block_uninit(ext2_filsys fs, ext2fs_block_bitmap map,
> !(ext2fs_bg_flags_test(fs, group, EXT2_BG_BLOCK_UNINIT)))
> return;
>
> - blk = (group * fs->super->s_blocks_per_group) +
> - fs->super->s_first_data_block;
> + blk = ext2fs_group_first_block2(fs, group);
>
> ext2fs_super_and_bgd_loc2(fs, group, &super_blk,
> &old_desc_blk, &new_desc_blk, 0);
> @@ -56,8 +55,7 @@ static void check_block_uninit(ext2_filsys fs, ext2fs_block_bitmap map,
> for (i=0; i < fs->super->s_blocks_per_group; i++, blk++)
> ext2fs_fast_unmark_block_bitmap2(map, blk);
>
> - blk = (group * fs->super->s_blocks_per_group) +
> - fs->super->s_first_data_block;
> + blk = ext2fs_group_first_block2(fs, group);
> for (i=0; i < fs->super->s_blocks_per_group; i++, blk++) {
> if ((blk == super_blk) ||
> (old_desc_blk && old_desc_blocks &&
> diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
> index 4d6af88..9cc7cba 100644
> --- a/lib/ext2fs/extent.c
> +++ b/lib/ext2fs/extent.c
> @@ -934,8 +934,7 @@ errcode_t ext2fs_extent_node_split(ext2_extent_handle_t handle)
>
> if (log_flex)
> group = group & ~((1 << (log_flex)) - 1);
> - goal_blk = (group * handle->fs->super->s_blocks_per_group) +
> - handle->fs->super->s_first_data_block;
> + goal_blk = ext2fs_group_first_block2(handle->fs, group);
> }
> retval = ext2fs_alloc_block2(handle->fs, goal_blk, block_buf,
> &new_node_pblk);
> diff --git a/lib/ext2fs/mkjournal.c b/lib/ext2fs/mkjournal.c
> index c154d91..c636a97 100644
> --- a/lib/ext2fs/mkjournal.c
> +++ b/lib/ext2fs/mkjournal.c
> @@ -358,9 +358,7 @@ static errcode_t write_journal_inode(ext2_filsys fs, ext2_ino_t journal_ino,
> ext2fs_bg_free_blocks_count(fs, group))
> group = i;
>
> - es.goal = (fs->super->s_blocks_per_group * group) +
> - fs->super->s_first_data_block;
> -
> + es.goal = ext2fs_group_first_block2(fs, group);
> retval = ext2fs_block_iterate3(fs, journal_ino, BLOCK_FLAG_APPEND,
> 0, mkjournal_proc, &es);
> if (es.err) {
> diff --git a/resize/resize2fs.c b/resize/resize2fs.c
> index ac965ee..8fdf35c 100644
> --- a/resize/resize2fs.c
> +++ b/resize/resize2fs.c
> @@ -492,9 +492,8 @@ retry:
> /*
> * Initialize the new block group descriptors
> */
> - group_block = fs->super->s_first_data_block +
> - old_fs->group_desc_count * fs->super->s_blocks_per_group;
> -
> + group_block = ext2fs_group_first_block2(fs,
> + old_fs->group_desc_count);
> csum_flag = EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
> EXT4_FEATURE_RO_COMPAT_GDT_CSUM);
> if (access("/sys/fs/ext4/features/lazy_itable_init", F_OK) == 0)
> @@ -651,9 +650,8 @@ static errcode_t adjust_superblock(ext2_resize_t rfs, blk64_t new_size)
> goto errout;
>
> memset(rfs->itable_buf, 0, fs->blocksize * fs->inode_blocks_per_group);
> - group_block = fs->super->s_first_data_block +
> - rfs->old_fs->group_desc_count * fs->super->s_blocks_per_group;
> -
> + group_block = ext2fs_group_first_block2(fs,
> + rfs->old_fs->group_desc_count);
> adj = rfs->old_fs->group_desc_count;
> max_group = fs->group_desc_count - adj;
> if (rfs->progress) {
>