e2image: double free when restoring image
PATCH 1/1 e2image: double free when restoring image
Hello,
I've been running into a double free when trying to apply an e2image to a loopback device:
# e2image /dev/sda1 sda1.img e2image 1.43-WIP (8-Jul-2013) # dd if=/dev/zero of=./lofile bs=1M seek=1k count=1 1+0 records in 1+0 records out 1048576 bytes (1.0 MB) copied, 0.00131481 s, 798 MB/s # losetup /dev/loop0 ./lofile # e2image -I /dev/loop0 ./sda1.img e2image 1.43-WIP (8-Jul-2013) *** glibc detected *** e2image: double free or corruption (!prev): 0x00000000011c3fd0 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x75296)[0x7f107bf62296] e2image[0x4125ab] e2image[0x408674] e2image[0x40448c] /lib64/libc.so.6(__libc_start_main+0xfd)[0x7f107bf0bcdd] e2image[0x401ce9] ======= Memory map: ======== 00400000-00425000 r-xp 00000000 fd:00 8907 /sbin/e2image 00625000-00626000 rw-p 00025000 fd:00 8907 /sbin/e2image 011b1000-011f3000 rw-p 00000000 00:00 0 [heap] 7f1075e46000-7f1075e5c000 r-xp 00000000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f1075e5c000-7f107605b000 ---p 00016000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f107605b000-7f107605c000 rw-p 00015000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f107605c000-7f107beed000 r--p 00000000 fd:00 3172 /usr/lib/locale/locale-archive 7f107beed000-7f107c073000 r-xp 00000000 fd:00 3189 /lib64/libc-2.12.so 7f107c073000-7f107c273000 ---p 00186000 fd:00 3189 /lib64/libc-2.12.so 7f107c273000-7f107c277000 r--p 00186000 fd:00 3189 /lib64/libc-2.12.so 7f107c277000-7f107c278000 rw-p 0018a000 fd:00 3189 /lib64/libc-2.12.so 7f107c278000-7f107c27d000 rw-p 00000000 00:00 0 7f107c27d000-7f107c294000 r-xp 00000000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c294000-7f107c493000 ---p 00017000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c493000-7f107c494000 r--p 00016000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c494000-7f107c495000 rw-p 00017000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c495000-7f107c499000 rw-p 00000000 00:00 0 7f107c499000-7f107c4b9000 r-xp 00000000 fd:00 3182 /lib64/ld-2.12.so 7f107c6ad000-7f107c6b0000 rw-p 00000000 00:00 0 7f107c6b6000-7f107c6b8000 rw-p 00000000 00:00 0 7f107c6b8000-7f107c6b9000 r--p 0001f000 fd:00 3182 /lib64/ld-2.12.so 7f107c6b9000-7f107c6ba000 rw-p 00020000 fd:00 3182 /lib64/ld-2.12.so 7f107c6ba000-7f107c6bb000 rw-p 00000000 00:00 0 7fffa93b9000-7fffa93ce000 rw-p 00000000 00:00 0 [stack] 7fffa93ff000-7fffa9400000 r-xp 00000000 00:00 0 [vdso] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall] Aborted
It appears to be due to a mismatch between the IO channel block size and the FS block size. ext2fs_rewrite_to_io is resetting the fs->io to be the IO channel of the new device, but that device still has the default unix IO channel block size of 1k. I have included a patch to copy the old IO block size into the new IO blocksize, which seems to solve the double free.
Thanks,
Kit
lib/ext2fs/openfs.c +1 -0
@@ -479,6 +479,7 @@ errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, io_channel new_io) {if ((fs->flags &EXT2_FLAG_IMAGE_FILE ) ==0 )return EXT2_ET_NOT_IMAGE_FILE ; + new_io->block_size = fs->io->block_size; fs->io = fs->image_io = new_io; fs->flags |=EXT2_FLAG_DIRTY |EXT2_FLAG_RW |EXT2_FLAG_BB_DIRTY |EXT2_FLAG_IB_DIRTY ;
Kit, thanks for sending the patch to the list.
You'll need to add a Signed-off-by: line. You can also add Reviewed-by: from me as well.
Cheers, Andreas
Quoted 73 lines · Kit Westneat
Hello,
I've been running into a double free when trying to apply an e2image to a loopback device:
# e2image /dev/sda1 sda1.img e2image 1.43-WIP (8-Jul-2013) # dd if=/dev/zero of=./lofile bs=1M seek=1k count=1 1+0 records in 1+0 records out 1048576 bytes (1.0 MB) copied, 0.00131481 s, 798 MB/s # losetup /dev/loop0 ./lofile # e2image -I /dev/loop0 ./sda1.img e2image 1.43-WIP (8-Jul-2013) *** glibc detected *** e2image: double free or corruption (!prev): 0x00000000011c3fd0 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x75296)[0x7f107bf62296] e2image[0x4125ab] e2image[0x408674] e2image[0x40448c] /lib64/libc.so.6(__libc_start_main+0xfd)[0x7f107bf0bcdd] e2image[0x401ce9] ======= Memory map: ======== 00400000-00425000 r-xp 00000000 fd:00 8907 /sbin/e2image 00625000-00626000 rw-p 00025000 fd:00 8907 /sbin/e2image 011b1000-011f3000 rw-p 00000000 00:00 0 [heap] 7f1075e46000-7f1075e5c000 r-xp 00000000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f1075e5c000-7f107605b000 ---p 00016000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f107605b000-7f107605c000 rw-p 00015000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f107605c000-7f107beed000 r--p 00000000 fd:00 3172 /usr/lib/locale/locale-archive 7f107beed000-7f107c073000 r-xp 00000000 fd:00 3189 /lib64/libc-2.12.so 7f107c073000-7f107c273000 ---p 00186000 fd:00 3189 /lib64/libc-2.12.so 7f107c273000-7f107c277000 r--p 00186000 fd:00 3189 /lib64/libc-2.12.so 7f107c277000-7f107c278000 rw-p 0018a000 fd:00 3189 /lib64/libc-2.12.so 7f107c278000-7f107c27d000 rw-p 00000000 00:00 0 7f107c27d000-7f107c294000 r-xp 00000000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c294000-7f107c493000 ---p 00017000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c493000-7f107c494000 r--p 00016000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c494000-7f107c495000 rw-p 00017000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c495000-7f107c499000 rw-p 00000000 00:00 0 7f107c499000-7f107c4b9000 r-xp 00000000 fd:00 3182 /lib64/ld-2.12.so 7f107c6ad000-7f107c6b0000 rw-p 00000000 00:00 0 7f107c6b6000-7f107c6b8000 rw-p 00000000 00:00 0 7f107c6b8000-7f107c6b9000 r--p 0001f000 fd:00 3182 /lib64/ld-2.12.so 7f107c6b9000-7f107c6ba000 rw-p 00020000 fd:00 3182 /lib64/ld-2.12.so 7f107c6ba000-7f107c6bb000 rw-p 00000000 00:00 0 7fffa93b9000-7fffa93ce000 rw-p 00000000 00:00 0 [stack] 7fffa93ff000-7fffa9400000 r-xp 00000000 00:00 0 [vdso] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall] AbortedIt appears to be due to a mismatch between the IO channel block size and the FS block size.
ext2fs_rewrite_to_iois resetting thefs->ioto be the IO channel of the new device, but that device still has the default unix IO channel block size of 1k. I have included a patch to copy the old IO block size into the new IO blocksize, which seems to solve the double free.Thanks,
Kitlib/ext2fs/openfs.c +1
-0@@ -479,6 +479,7 @@ errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, io_channel new_io) {if ((fs->flags &EXT2_FLAG_IMAGE_FILE ) ==0 )return EXT2_ET_NOT_IMAGE_FILE ; + new_io->block_size = fs->io->block_size; fs->io = fs->image_io = new_io; fs->flags |=EXT2_FLAG_DIRTY |EXT2_FLAG_RW |EXT2_FLAG_BB_DIRTY |EXT2_FLAG_IB_DIRTY ;
Kit Westneat
Reviewed-by Andreas Dilgere2fsprogs: copy fs block size to new io
e2image manually opens a new IO channel, which uses the default block size of 1k. This patch sets the new IO channel's block size to match the fs block size.
---
lib/ext2fs/openfs.c +1 -0
@@ -441,6 +441,7 @@ errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, io_channel new_io) {if ((fs->flags &EXT2_FLAG_IMAGE_FILE ) ==0 )return EXT2_ET_NOT_IMAGE_FILE ; + new_io->block_size = fs->io->block_size; fs->io = fs->image_io = new_io; fs->flags |=EXT2_FLAG_DIRTY |EXT2_FLAG_RW |EXT2_FLAG_BB_DIRTY |EXT2_FLAG_IB_DIRTY ;
---
Kit Westneat
L3 Lustre Support, DDN
703-659-3869
Quoted 81 lines · Andreas
Kit, thanks for sending the patch to the list.
You'll need to add a Signed-off-by: line. You can also add Reviewed-by: from me as well.
Cheers, Andreas
Quoted 73 lines · Kit Westneat
Hello,
I've been running into a double free when trying to apply an e2image to a loopback device:
# e2image /dev/sda1 sda1.img e2image 1.43-WIP (8-Jul-2013) # dd if=/dev/zero of=./lofile bs=1M seek=1k count=1 1+0 records in 1+0 records out 1048576 bytes (1.0 MB) copied, 0.00131481 s, 798 MB/s # losetup /dev/loop0 ./lofile # e2image -I /dev/loop0 ./sda1.img e2image 1.43-WIP (8-Jul-2013) *** glibc detected *** e2image: double free or corruption (!prev): 0x00000000011c3fd0 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x75296)[0x7f107bf62296] e2image[0x4125ab] e2image[0x408674] e2image[0x40448c] /lib64/libc.so.6(__libc_start_main+0xfd)[0x7f107bf0bcdd] e2image[0x401ce9] ======= Memory map: ======== 00400000-00425000 r-xp 00000000 fd:00 8907 /sbin/e2image 00625000-00626000 rw-p 00025000 fd:00 8907 /sbin/e2image 011b1000-011f3000 rw-p 00000000 00:00 0 [heap] 7f1075e46000-7f1075e5c000 r-xp 00000000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f1075e5c000-7f107605b000 ---p 00016000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f107605b000-7f107605c000 rw-p 00015000 fd:00 50 /lib64/libgcc_s-4.4.6-20110824.so.1 7f107605c000-7f107beed000 r--p 00000000 fd:00 3172 /usr/lib/locale/locale-archive 7f107beed000-7f107c073000 r-xp 00000000 fd:00 3189 /lib64/libc-2.12.so 7f107c073000-7f107c273000 ---p 00186000 fd:00 3189 /lib64/libc-2.12.so 7f107c273000-7f107c277000 r--p 00186000 fd:00 3189 /lib64/libc-2.12.so 7f107c277000-7f107c278000 rw-p 0018a000 fd:00 3189 /lib64/libc-2.12.so 7f107c278000-7f107c27d000 rw-p 00000000 00:00 0 7f107c27d000-7f107c294000 r-xp 00000000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c294000-7f107c493000 ---p 00017000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c493000-7f107c494000 r--p 00016000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c494000-7f107c495000 rw-p 00017000 fd:00 3213 /lib64/libpthread-2.12.so 7f107c495000-7f107c499000 rw-p 00000000 00:00 0 7f107c499000-7f107c4b9000 r-xp 00000000 fd:00 3182 /lib64/ld-2.12.so 7f107c6ad000-7f107c6b0000 rw-p 00000000 00:00 0 7f107c6b6000-7f107c6b8000 rw-p 00000000 00:00 0 7f107c6b8000-7f107c6b9000 r--p 0001f000 fd:00 3182 /lib64/ld-2.12.so 7f107c6b9000-7f107c6ba000 rw-p 00020000 fd:00 3182 /lib64/ld-2.12.so 7f107c6ba000-7f107c6bb000 rw-p 00000000 00:00 0 7fffa93b9000-7fffa93ce000 rw-p 00000000 00:00 0 [stack] 7fffa93ff000-7fffa9400000 r-xp 00000000 00:00 0 [vdso] ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall] AbortedIt appears to be due to a mismatch between the IO channel block size and the FS block size.
ext2fs_rewrite_to_iois resetting thefs->ioto be the IO channel of the new device, but that device still has the default unix IO channel block size of 1k. I have included a patch to copy the old IO block size into the new IO blocksize, which seems to solve the double free.Thanks,
Kitlib/ext2fs/openfs.c +1
-0@@ -479,6 +479,7 @@ errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, io_channel new_io) {if ((fs->flags &EXT2_FLAG_IMAGE_FILE ) ==0 )return EXT2_ET_NOT_IMAGE_FILE ; + new_io->block_size = fs->io->block_size; fs->io = fs->image_io = new_io; fs->flags |=EXT2_FLAG_DIRTY |EXT2_FLAG_RW |EXT2_FLAG_BB_DIRTY |EXT2_FLAG_IB_DIRTY ;
lib/ext2fs/openfs.c errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, · line 445
Quoted 10 lines · Kit Westneat
lib/ext2fs/openfs.c +1
-0@@ -441,6 +441,7 @@ errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, io_channel new_io) {if ((fs->flags &EXT2_FLAG_IMAGE_FILE ) ==0 )return EXT2_ET_NOT_IMAGE_FILE ; + new_io->block_size = fs->io->block_size;
One other thought; we should really use io_channel_set_blksize(), which is the supported interface to set the blocksize. This makes sure that if there is any cached blocks, they are flushed, and there might also some other io_channels, either now in the future (i.e., to talk to Windows, etc.) where some kind of ioctl or other io control functoin might be needed to set the block size.
I'll send out a corrected patch.
Thanks again, for looking into this!
- Ted
PATCH 1/3 e2image: pass the correct size write_header
Commit bf0449b1a654, which added the ability to write qcow2 files, generalized the write_header() file to take the size of the header structure which it writes out. Unfortunately, it changed the call which supported original e2image format to pass in fs->blocksize, instead of the actual size of the e2image header structure (which is substantially smaller than fs->blocksize). This meant that we copied in stack garbage into the e2image file, and it made valgrind quite unhappy.
--- misc/e2image.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
misc/e2image.c +2 -2
@@ -165,7 +165,7 @@ static void write_image_file(ext2_filsys fs, int fd)struct stat st;errcode_t retval; -write_header (fd,NULL , fs->blocksize, fs->blocksize); +write_header (fd,NULL ,sizeof (struct ext2_image_hdr ), fs->blocksize);memset (&hdr,0 ,sizeof (struct ext2_image_hdr )); hdr.offset_super =ext2fs_llseek (fd,0 ,SEEK_CUR );
@@ -214,7 +214,7 @@ static void write_image_file(ext2_filsys fs, int fd)memcpy (hdr.fs_uuid, fs->super->s_uuid,sizeof (hdr.fs_uuid)); hdr.image_time =time (0 ); -write_header (fd, &hdr, fs->blocksize, fs->blocksize); +write_header (fd, &hdr,sizeof (struct ext2_image_hdr ), fs->blocksize); }/*
Signature · 1 line
1.8.5.rc3.362.gdf10213
PATCH 2/3 libext2fs: set the fs block size to new_io in ext2fs_rewrite_to_io()
From: Kit Westneat <[email protected]>
e2image manually opens a new IO channel, and then sets the file system to use this new IO channel via ext2fs_rewrite+to_io(). We need to make sure the IO channel is set to the file system's block size to avoid some nasty buffer overruns.
[ Modified by tytso to use io_channel_set_blksize() ]
--- lib/ext2fs/openfs.c | 5 +++++ 1 file changed, 5 insertions(+)
lib/ext2fs/openfs.c +5 -0
@@ -451,8 +451,13 @@ errcode_t ext2fs_set_data_io(ext2_filsys fs, io_channel new_io)errcode_t ext2fs_rewrite_to_io (ext2_filsys fs,io_channel new_io) { +errcode_t err; +if ((fs->flags &EXT2_FLAG_IMAGE_FILE ) ==0 )return EXT2_ET_NOT_IMAGE_FILE ; + err =io_channel_set_blksize (new_io, fs->blocksize); +if (err) +return err; fs->io = fs->image_io = new_io; fs->flags |=EXT2_FLAG_DIRTY |EXT2_FLAG_RW |EXT2_FLAG_BB_DIRTY |EXT2_FLAG_IB_DIRTY ;
Signature · 1 line
1.8.5.rc3.362.gdf10213
PATCH 3/3 libext2fs: fix some memory leaks with image file handling
In reply to Theodore Ts'oThese memory leaks were discovered by using "valgrind --leak-check=full" while running "e2image -I bar.img foo.e2i"
--- lib/ext2fs/freefs.c | 2 ++ lib/ext2fs/openfs.c | 7 +++++++ 2 files changed, 9 insertions(+)
lib/ext2fs/freefs.c +2 -0
@@ -43,6 +43,8 @@ void ext2fs_free(ext2_filsys fs)ext2fs_free_block_bitmap (fs->block_map);if (fs->inode_map)ext2fs_free_inode_bitmap (fs->inode_map); +if (fs->image_header) +ext2fs_free_mem (&fs->image_header);if (fs->badblocks)ext2fs_badblocks_list_free (fs->badblocks);
lib/ext2fs/openfs.c +7 -0
@@ -458,6 +458,13 @@ errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, io_channel new_io) err =io_channel_set_blksize (new_io, fs->blocksize);if (err)return err; +if ((new_io == fs->image_io) || (new_io == fs->io)) +return 0 ; +if ((fs->image_io != fs->io) && + fs->image_io) +io_channel_close (fs->image_io); +if (fs->io) +io_channel_close (fs->io); fs->io = fs->image_io = new_io; fs->flags |=EXT2_FLAG_DIRTY |EXT2_FLAG_RW |EXT2_FLAG_BB_DIRTY |EXT2_FLAG_IB_DIRTY ;
Signature · 1 line
1.8.5.rc3.362.gdf10213
Quoted 4 lines · Kit Westneat
Hello,
I've been running into a double free when trying to apply an e2image to a loopback device:
It looks like there are a number of memory pointer overruns which valgrind is finding, not just the one you have pointed out. Thanks for pointing out this issue.
For this particular case, it's probably better to set new_io->block_size to fs->blocksize; I'll send out some patches to fix these issues, and very clearly we need to add some regression tests to catch these in the future.
Thanks,
- Ted