ext2 inode size patch - RE: PR kern/124621

[Stanislav Sedov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, 3 Dec 2008 17:53:43 -0500
"Josh Carroll" <josh.carroll at gmail.com> 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 <linux/ext2_fs_sb.h>
> 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:497362c8967008581431178!