Re: Bad ext3/nfs DoS bug

[Andrew Morton <akpm@osdl.org>]

On Sat, 22 Jul 2006 09:17:59 -0400
Theodore Tso <tytso@mit.edu> wrote:

> Sorry, OLS and some work-related emergencies have been hitting hard
> this week, so I've been deferred doing a full review of Jan's patch.
> Hopefully after OLS I'll be able to comment more fully.
> 
> On Fri, Jul 21, 2006 at 05:06:27PM -0700, Andrew Morton wrote:
> > There are strange things happening in here.
> > 
> > > +static inline int ext3_valid_inum(struct super_block *sb, unsigned long ino)
> > > +{
> > > +	return ino == EXT3_ROOT_INO ||
> > > +		ino == EXT3_JOURNAL_INO ||
> > > +		ino == EXT3_RESIZE_INO ||
> > > +		(ino > EXT3_FIRST_INO(sb) &&
> > > +		 ino <= le32_to_cpu(EXT3_SB(sb)->s_es->s_inodes_count));
> > > +}
> > 
> > One would expect the inode validity test to be
> > 
> > 	(ino >= EXT3_FIRST_INO(sb)) && (ino < ...->s_inodes_count))
> >
> > but even this assumes that s_inodes_count is misnamed and really should be
> > called s_last_inode_plus_one.  If it is not misnamed then the validity test
> > should be
> > 
> > 	(ino >= EXT3_FIRST_INO(sb)) &&
> > 		(ino < EXT3_FIRST_INO(sb) + ...->s_inodes_count))
> > 
> > Look through the filesystem for other uses of EXT3_FIRST_INO().  It's all
> > rather fishily inconsistent.
> 
> I don't think there's anything in consistent.  Basically, inodes are 1
> based (inode number 0 in a directory entry means that the file is
> deleted and the directory entry is freed).  Hence valid inode numbers
> are between 1 and s_inodes_count, inclusive.
> 
> Inodes between 1 and EXT3_FIRST_INO-1 (inclusive) are reserved, are
> should always be marked as in use in the inode allocation bitmap.
> EXT3_FIRST_INO (which is 11 on all ext3 filesystems to date) is
> generally the lost+found directory, but that's just because it is the
> first file/directory which is allocated by mke2fs.  So EXT3_FIRST_INO
> representes the first inode which can be allocated by userspace.  (The
> root inode doesn't fall in this category because it can never be
> deleted or reallocated after the filesystem is created, and as a nod
> to Unix filesystem history, it has inode #2).
> 
> The net of all of this is the inode validity test should be:
> 
>  	(ino >= EXT3_FIRST_INO(sb)) && (ino <= ...->s_inodes_count))

I agree; I made that change.

> However, I would suggest that we *not* allow remote NFS users to get
> access to the journal inode or the resize inode, please?  That's only
> going to cause mischief of the DoS attack kind.....  

<looks at Neil>

<then looks at ext2>