From owner-freebsd-stable@FreeBSD.ORG Sun Jan 18 17:11:38 2009 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B5B111065674 for ; Sun, 18 Jan 2009 17:11:38 +0000 (UTC) (envelope-from stas@FreeBSD.org) Received: from mx0.deglitch.com (backbone.deglitch.com [IPv6:2001:16d8:fffb:4::abba]) by mx1.freebsd.org (Postfix) with ESMTP id 397E38FC1A for ; Sun, 18 Jan 2009 17:11:38 +0000 (UTC) (envelope-from stas@FreeBSD.org) Received: from DSPAM-Daemon (localhost [127.0.0.1]) by mx0.deglitch.com (Postfix) with SMTP id 0B33F8FC51 for ; Sun, 18 Jan 2009 20:11:36 +0300 (MSK) Received: from orion.SpringDaemons.com (drsun1.dialup.corbina.ru [85.21.245.235]) by mx0.deglitch.com (Postfix) with ESMTPA id B8A8B8FC18; Sun, 18 Jan 2009 20:11:35 +0300 (MSK) Received: from orion (localhost [127.0.0.1]) by orion.SpringDaemons.com (Postfix) with SMTP id DB77B398F3; Sun, 18 Jan 2009 20:11:38 +0300 (MSK) Date: Sun, 18 Jan 2009 20:11:38 +0300 From: Stanislav Sedov To: josh.carroll@gmail.com Message-Id: <20090118201138.eeb5d9ad.stas@FreeBSD.org> In-Reply-To: <8cb6106e0812031453j6dc2f2f4i374145823c084eaa@mail.gmail.com> References: <8cb6106e0811241129o642dcf28re4ae177c8ccbaa25@mail.gmail.com> <20081125140601.GH2042@deviant.kiev.zoral.com.ua> <8cb6106e0811250617q5fffb41exe20dfb8314fc4a9d@mail.gmail.com> <20081125142827.GI2042@deviant.kiev.zoral.com.ua> <8cb6106e0811250657q6fdf08b0x1e94f35fd0a7ed4f@mail.gmail.com> <20081125150342.GL2042@deviant.kiev.zoral.com.ua> <8cb6106e0812031453j6dc2f2f4i374145823c084eaa@mail.gmail.com> Organization: The FreeBSD Project X-XMPP: ssedov@jabber.ru X-Voice: +7 916 849 20 23 X-PGP-Fingerprint: F21E D6CC 5626 9609 6CE2 A385 2BF5 5993 EB26 9581 X-Mailer: carrier-pigeon Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-DSPAM-Result: Innocent X-DSPAM-Processed: Sun Jan 18 20:11:36 2009 X-DSPAM-Confidence: 1.0000 X-DSPAM-Improbability: 1 in 98689409 chance of being spam X-DSPAM-Probability: 0.0023 X-DSPAM-Signature: 497362c8967002668918391 Cc: Kostik Belousov , freebsd-fs@freebsd.org, Stable , FreeBSD Subject: Re: ext2 inode size patch - RE: PR kern/124621 X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Jan 2009 17:11:39 -0000 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Wed, 3 Dec 2008 17:53:43 -0500 "Josh Carroll" mentioned: > > Ok, I describe my concern once more. I do not object against the checking > > of the inode size. But, if inode size is changed, then some data is added > > to the inode, that could (and usually does, otherwise why extend it ?) > > change intrerpetation of the inode. Thus, we need a verification of the > > fact that simply ignoring added fields does not damage filesystem or > > cause user data corruption. Verification != testing. > > Let me take another crack at explaining why I think this patch is not dangerous. > > The inode size is determined by reading the following member: > > __u16 s_inode_size; > > of the ext2_super_block structure. > > I took a look at the Linux 2.6.27.7 kernel source, and indeed they do > something very similar if not the same: > > #define EXT2_INODE_SIZE(s) (EXT2_SB(s)->s_inode_size) > > If you compare to what I did: > > #define EXT2_INODE_SIZE(s) ((s)->u.ext2_sb.s_inode_size) > > This is really the same thing, since EXT2_SB is defined (in the Linux > kernel src as): > > #ifdef __KERNEL__ > #include > static inline struct ext2_sb_info *EXT2_SB(struct super_block *sb) > { > return sb->s_fs_info; > } > > And struct ext2_sb_info has the following member: > > int s_inode_size; > > Essentially, the changes I made simply query the existing information > from the filesystem, which is what the Linux kernel does as well. > > The inode size, blocks per group, etc are all defined at filesystem > creation time by mke2fs and it ensures the sanity of the relationship > between the inodes/blocks/block groups. > > The first diagram here: > > http://sunsite.nus.sg/LDP/LDP/tlk/node95.html > > Makes it clear that as long as the number of inodes per block and the > blocks per group is is sane during fs creation, looking up the inode > size as my patch does is fine, since the creation of the filesystem is > ensures a correct by construction situation. We're simply reading the > size of the inode based on the filesystem. > > I hope this is sufficient to convince some further thought about > committing this. > > For those interested in the relevant Linux kernel source, you can have > a look at line 105 of include/linux/ext2_fs.h from the linux-2.6.27.7 > kernel source. > Hi Josh! I've commited the similar patch today that should be fixing your problem. Can you check this, please? Sorry I've missed this thread in the first place. - -- Stanislav Sedov ST4096-RIPE -----BEGIN PGP SIGNATURE----- iEYEARECAAYFAklzYsoACgkQK/VZk+smlYF3ZwCeOyVUdzrKOdu4Pg3ztAZ0QQaY GGIAnA+oL054T0EAajbfwpYSTDRKVISC =jJFT -----END PGP SIGNATURE----- !DSPAM:497362c8967002668918391!