[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20060724185604.9181714c.akpm@osdl.org>
Date: Mon, 24 Jul 2006 18:56:04 -0700
From: Andrew Morton <akpm@...l.org>
To: Theodore Tso <tytso@....edu>
Cc: neilb@...e.de, jack@...e.cz, 20@...ingley.org, marcel@...tmann.org,
linux-kernel@...r.kernel.org, sct@...hat.com, adilger@...sterfs.com
Subject: Re: Bad ext3/nfs DoS bug
On Sat, 22 Jul 2006 09:17:59 -0400
Theodore Tso <tytso@....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>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists