Re: Bad ext3/nfs DoS bug

Andrew Morton
2006-07-25T01:56:58Z

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>