lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 14 Nov 2014 16:57:06 +0800 From: Xiaoguang Wang <wangxg.fnst@...fujitsu.com> To: "Darrick J. Wong" <darrick.wong@...cle.com> CC: <linux-ext4@...r.kernel.org>, <tytso@....edu> Subject: Re: [PATCH v2 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size Hi, On 11/13/2014 06:39 AM, Darrick J. Wong wrote: > On Wed, Nov 12, 2014 at 05:49:39PM +0800, Xiaoguang Wang wrote: >> When we use tune2fs -I new_ino_size to change inode size, if everything is OK, >> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so >> obviously, we need to re-compute the group descriptor checksums, fix this. If >> not doing this, mount operation will fail. >> >> Meanwhile, the patch will trigger an existing memory write overflow, which will >> casue segfault, please see the next patch. >> >> Signed-off-by: Xiaoguang Wang <wangxg.fnst@...fujitsu.com> >> --- >> misc/tune2fs.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/misc/tune2fs.c b/misc/tune2fs.c >> index 065b483..91dc7c1 100644 >> --- a/misc/tune2fs.c >> +++ b/misc/tune2fs.c >> @@ -2908,8 +2908,7 @@ retry_open: >> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM)) >> rewrite_checksums = 1; >> } >> - if (rewrite_checksums) >> - rewrite_metadata_checksums(fs); >> + >> if (I_flag) { >> if (mount_flags & EXT2_MF_MOUNTED) { >> fputs(_("The inode size may only be " >> @@ -2935,6 +2934,7 @@ retry_open: >> if (resize_inode(fs, new_inode_size) == 0) { >> printf(_("Setting inode size %lu\n"), >> new_inode_size); >> + rewrite_checksums = 1; >> } else { >> printf("%s", _("Failed to change inode size\n")); >> rc = 1; >> @@ -2942,6 +2942,9 @@ retry_open: >> } >> } >> >> + if (rewrite_checksums) >> + rewrite_metadata_checksums(fs); >> + > > Aha! expand_inode_table() fails to recompute the checksums of the inode blocks > it's moving around, and happily your change takes care of recomputing the inode > checksums for a metadata_csum FS. The changelog for this patch doesn't mention > this, but it should. Oh, I had not realized the inode checksum, thanks for pointing this. So we agree to call a rewrite_metadata_checksums() here, in new version patch, i'll update the changelog :) > > There ought to be a regression test for this. Can you send one along, please? Sure, I spent some time about how to write a test, will send this test soon. Regards, Xiaoguang Wang > > I crafted my own test case for the metadata_csum failure while trying to figure > out what this patchset does, so I'll simply send it out. Unfortunately, the > test requires a fix for a bug in the hugefile code that I'll cram onto the > patchbomb. > > Other than that, you can add for all three patches: > Reviewed-by: Darrick J. Wong <darrick.wong@...cle.com> > > --D > >> if (l_flag) >> list_super(sb); >> if (stride_set) { >> -- >> 1.8.2.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in >> the body of a message to majordomo@...r.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > . > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists