Patchwork [3/4] e2fsprogs: Fix write size in ext2fs_mmp_write

login
register
mail settings
Submitter Eric Sandeen
Date Nov. 10, 2011, 11 p.m.
Message ID <4EBC577C.9010607@redhat.com>
Download mbox | patch
Permalink /patch/125014/
State Accepted
Headers show

Comments

Eric Sandeen - Nov. 10, 2011, 11 p.m.
Without this change, we will write data past the end of the
mmp buf.  Valgrind catches this:

==6373== Syscall param write(buf) points to unaddressable byte(s)
==6373==    at 0x362260E470: __write_nocancel (in /lib64/libpthread-2.12.2.so)
==6373==    by 0x41CF83: raw_write_blk (unix_io.c:255)
==6373==    by 0x41D2BC: unix_write_blk64 (unix_io.c:757)
==6373==    by 0x41A05D: ext2fs_mmp_write (mmp.c:130)
==6373==    by 0x40B0C9: do_set_mmp_value (set_fields.c:806)
==6373==    by 0x421B61: really_execute_command (execute_cmd.c:108)
==6373==    by 0x421C54: ss_execute_line (execute_cmd.c:234)
==6373==    by 0x403743: main (debugfs.c:2339)
==6373==  Address 0x63f000 is not stack'd, malloc'd or (recently) free'd

and in my testing it led to silent failures while writing the mmp
block in debugfs:

write(3, "xV4\22PMM\342\325V\274N\0\0\0\0host.name."..., 4096) = -1 EFAULT (Bad address)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

p.s. I could do with a comment about what a negative "count" means...?



--
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 - Nov. 12, 2011, 2:13 a.m.
On Thu, Nov 10, 2011 at 05:00:12PM -0600, Eric Sandeen wrote:
> Without this change, we will write data past the end of the
> mmp buf.  Valgrind catches this:
> 
> ==6373== Syscall param write(buf) points to unaddressable byte(s)
> ==6373==    at 0x362260E470: __write_nocancel (in /lib64/libpthread-2.12.2.so)
> ==6373==    by 0x41CF83: raw_write_blk (unix_io.c:255)
> ==6373==    by 0x41D2BC: unix_write_blk64 (unix_io.c:757)
> ==6373==    by 0x41A05D: ext2fs_mmp_write (mmp.c:130)
> ==6373==    by 0x40B0C9: do_set_mmp_value (set_fields.c:806)
> ==6373==    by 0x421B61: really_execute_command (execute_cmd.c:108)
> ==6373==    by 0x421C54: ss_execute_line (execute_cmd.c:234)
> ==6373==    by 0x403743: main (debugfs.c:2339)
> ==6373==  Address 0x63f000 is not stack'd, malloc'd or (recently) free'd
> 
> and in my testing it led to silent failures while writing the mmp
> block in debugfs:
> 
> write(3, "xV4\22PMM\342\325V\274N\0\0\0\0host.name."..., 4096) = -1 EFAULT (Bad address)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Applied, thanks.

						- 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

Patch

diff --git a/lib/ext2fs/mmp.c b/lib/ext2fs/mmp.c
index 91f4fb2..b27d9a4 100644
--- a/lib/ext2fs/mmp.c
+++ b/lib/ext2fs/mmp.c
@@ -127,7 +127,7 @@  errcode_t ext2fs_mmp_write(ext2_filsys fs, blk64_t mmp_blk, void *buf)
 
 	/* I was tempted to make this use O_DIRECT and the mmp_fd, but
 	 * this caused no end of grief, while leaving it as-is works. */
-	retval = io_channel_write_blk64(fs->io, mmp_blk, -fs->blocksize, buf);
+	retval = io_channel_write_blk64(fs->io, mmp_blk, -(int)sizeof(struct mmp_struct), buf);
 
 #ifdef WORDS_BIGENDIAN
 	ext2fs_swap_mmp(mmp_s);