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
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 2ad9114..69660ff 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -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
On 2013-11-27, at 14:33, "Kit Westneat" <[email protected]> wrote:
> 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
>
> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
> index 2ad9114..69660ff 100644
> --- a/lib/ext2fs/openfs.c
> +++ b/lib/ext2fs/openfs.c
> @@ -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;
>
e2fsprogs: 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.
Signed-off-by: Kit Westneat <[email protected]>
Reviewed-by: Andreas Dilger <[email protected]>
---
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 113b80e..6861cfe 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -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
On 11/29/2013 12:45 PM, Dilger, Andreas wrote:
> 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
>
> On 2013-11-27, at 14:33, "Kit Westneat" <[email protected]> wrote:
>
>> 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
>>
>> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
>> index 2ad9114..69660ff 100644
>> --- a/lib/ext2fs/openfs.c
>> +++ b/lib/ext2fs/openfs.c
>> @@ -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;
>>
On Wed, Nov 27, 2013 at 04:32:38PM -0500, Kit Westneat wrote:
> 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
On Mon, Dec 02, 2013 at 12:27:03PM -0500, Kit Westneat wrote:
> diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
> index 113b80e..6861cfe 100644
> --- a/lib/ext2fs/openfs.c
> +++ b/lib/ext2fs/openfs.c
> @@ -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
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.
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
misc/e2image.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/misc/e2image.c b/misc/e2image.c
index 4a5bb22..aa363fb 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -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);
}
/*
--
1.8.5.rc3.362.gdf10213
These memory leaks were discovered by using "valgrind
--leak-check=full" while running "e2image -I bar.img foo.e2i"
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
lib/ext2fs/freefs.c | 2 ++
lib/ext2fs/openfs.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/lib/ext2fs/freefs.c b/lib/ext2fs/freefs.c
index 28c4132..1ad2d91 100644
--- a/lib/ext2fs/freefs.c
+++ b/lib/ext2fs/freefs.c
@@ -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);
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index c38b586..4cdbde1 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -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;
--
1.8.5.rc3.362.gdf10213
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() ]
Signed-off-by: Kit Westneat <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>
Reviewed-by: Andreas Dilger <[email protected]>
---
lib/ext2fs/openfs.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 9fe1645..c38b586 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -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;
--
1.8.5.rc3.362.gdf10213