Re: Bad ext3/nfs DoS bug

[Neil Brown <neilb@suse.de>]

On Monday July 24, akpm@osdl.org wrote:
> On Sat, 22 Jul 2006 09:17:59 -0400
> Theodore Tso <tytso@mit.edu> wrote:
> > 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.
> 

Yeh, my bad.  I double checked the second comparison but not the first
:-(

> > 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>

See, I had a funny feeling that someone was watching me ....

I doubt there is much room for a real problem here.  
To get to the point of IO on any of these files, you would need root
access to the whole filesystem anyway.

The follow patch should do what is suggested though.

> 
> <then looks at ext2>

Hmmm. are two looks allowed in the same email?

NeilBrown

----------------------------
Make ext3 reject filehandles referring to special files.

Inodes earlier than the 'first' inode (e.g. journal,
resize) should be rejected early - except the root inode.


Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./fs/exportfs/expfs.c |    4 +++-
 ./fs/ext3/super.c     |   15 +++++++++++++++
 ./include/linux/fs.h  |    2 ++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff .prev/fs/exportfs/expfs.c ./fs/exportfs/expfs.c
--- .prev/fs/exportfs/expfs.c	2006-07-25 12:19:21.000000000 +1000
+++ ./fs/exportfs/expfs.c	2006-07-25 12:16:12.000000000 +1000
@@ -392,7 +392,8 @@ out:
 }
 
 
-static struct dentry *export_iget(struct super_block *sb, unsigned long ino, __u32 generation)
+struct dentry *export_iget(struct super_block *sb, unsigned long ino,
+			   __u32 generation)
 {
 
 	/* iget isn't really right if the inode is currently unallocated!!
@@ -434,6 +435,7 @@ static struct dentry *export_iget(struct
 	}
 	return result;
 }
+EXPORT_SYMBOL_GPL(export_iget);
 
 
 static struct dentry *get_object(struct super_block *sb, void *vobjp)

diff .prev/fs/ext3/super.c ./fs/ext3/super.c
--- .prev/fs/ext3/super.c	2006-07-25 12:19:21.000000000 +1000
+++ ./fs/ext3/super.c	2006-07-25 12:19:43.000000000 +1000
@@ -554,6 +554,20 @@ static int ext3_show_options(struct seq_
 	return 0;
 }
 
+
+static struct dentry *ext3_get_dentry(struct super_block *sb, void *vobjp)
+{
+	__u32 *objp = vobjp;
+	unsigned long ino = objp[0];
+	__u32 generation = objp[1];
+
+	if (ino != EXT3_ROOT_INO && ino < EXT3_FIRST_INO(sb))
+		return ERR_PTR(-ESTALE);
+
+	return export_iget(sb, ino, generation);
+}
+
+
 #ifdef CONFIG_QUOTA
 #define QTYPE2NAME(t) ((t)==USRQUOTA?"user":"group")
 #define QTYPE2MOPT(on, t) ((t)==USRQUOTA?((on)##USRJQUOTA):((on)##GRPJQUOTA))
@@ -622,6 +636,7 @@ static struct super_operations ext3_sops
 
 static struct export_operations ext3_export_ops = {
 	.get_parent = ext3_get_parent,
+	.get_dentry = ext3_get_dentry,
 };
 
 enum {

diff .prev/include/linux/fs.h ./include/linux/fs.h
--- .prev/include/linux/fs.h	2006-07-25 12:19:21.000000000 +1000
+++ ./include/linux/fs.h	2006-07-25 12:15:32.000000000 +1000
@@ -1381,6 +1381,8 @@ extern struct dentry *
 find_exported_dentry(struct super_block *sb, void *obj, void *parent,
 		     int (*acceptable)(void *context, struct dentry *de),
 		     void *context);
+struct dentry *export_iget(struct super_block *sb, unsigned long ino,
+			   __u32 generation);
 
 struct file_system_type {
 	const char *name;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/