* [PATCH 000 of 2] knfsd: Don't allow bad file handles to cause extX to go readonly
@ 2006-07-28 0:31 ` NeilBrown
0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2006-07-28 0:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: nfs, linux-kernel
Currently, and file handle with a bad inode number in it can cause
ext2 to go to readonly (as it looks like a corrupted filesystem)
and could allow remote access to ext3 special files like the journal.
These patches give ext2/3 their own get_dentry method which checks the
inode number early before other bits of the code can be freaked out by
it.
These are revised versions of earlier patches. Rather than exporting
export_iget, we open code it and simplify it slightly. This avoids
and extra module dependancy.
NeilBrown
To follow:
[PATCH 001 of 2] knfsd: Have ext2 reject file handles with bad inode numbers early.
[PATCH 002 of 2] knfsd: Make ext3 reject filehandles referring to invalid inode numbers
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 001 of 2] knfsd: Have ext2 reject file handles with bad inode numbers early.
2006-07-28 0:31 ` NeilBrown
@ 2006-07-28 0:31 ` NeilBrown
-1 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2006-07-28 0:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: nfs, linux-kernel
This prevents bad inode numbers from triggering errors in
ext2_get_inode.
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/ext2/super.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff .prev/fs/ext2/super.c ./fs/ext2/super.c
--- .prev/fs/ext2/super.c 2006-07-28 10:12:55.000000000 +1000
+++ ./fs/ext2/super.c 2006-07-28 10:17:58.000000000 +1000
@@ -251,6 +251,46 @@ static struct super_operations ext2_sops
#endif
};
+static struct dentry *ext2_get_dentry(struct super_block *sb, void *vobjp)
+{
+ __u32 *objp = vobjp;
+ unsigned long ino = objp[0];
+ __u32 generation = objp[1];
+ struct inode *inode;
+ struct dentry *result;
+
+ if (ino != EXT2_ROOT_INO && ino < EXT2_FIRST_INO(sb))
+ return ERR_PTR(-ESTALE);
+ if (ino > le32_to_cpu(EXT2_SB(sb)->s_es->s_inodes_count))
+ return ERR_PTR(-ESTALE);
+
+ /* iget isn't really right if the inode is currently unallocated!!
+ * ext2_read_inode currently does appropriate checks, but
+ * it might be "neater" to call ext2_get_inode first and check
+ * if the inode is valid.....
+ */
+ inode = iget(sb, ino);
+ if (inode == NULL)
+ return ERR_PTR(-ENOMEM);
+ if (is_bad_inode(inode)
+ || (generation && inode->i_generation != generation)
+ ) {
+ /* we didn't find the right inode.. */
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
+ /* now to find a dentry.
+ * If possible, get a well-connected one
+ */
+ result = d_alloc_anon(inode);
+ if (!result) {
+ iput(inode);
+ return ERR_PTR(-ENOMEM);
+ }
+ return result;
+}
+
+
/* Yes, most of these are left as NULL!!
* A NULL value implies the default, which works with ext2-like file
* systems, but can be improved upon.
@@ -258,6 +298,7 @@ static struct super_operations ext2_sops
*/
static struct export_operations ext2_export_ops = {
.get_parent = ext2_get_parent,
+ .get_dentry = ext2_get_dentry,
};
static unsigned long get_sb_block(void **data)
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 001 of 2] knfsd: Have ext2 reject file handles with bad inode numbers early.
@ 2006-07-28 0:31 ` NeilBrown
0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2006-07-28 0:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: nfs, linux-kernel
This prevents bad inode numbers from triggering errors in
ext2_get_inode.
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/ext2/super.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff .prev/fs/ext2/super.c ./fs/ext2/super.c
--- .prev/fs/ext2/super.c 2006-07-28 10:12:55.000000000 +1000
+++ ./fs/ext2/super.c 2006-07-28 10:17:58.000000000 +1000
@@ -251,6 +251,46 @@ static struct super_operations ext2_sops
#endif
};
+static struct dentry *ext2_get_dentry(struct super_block *sb, void *vobjp)
+{
+ __u32 *objp = vobjp;
+ unsigned long ino = objp[0];
+ __u32 generation = objp[1];
+ struct inode *inode;
+ struct dentry *result;
+
+ if (ino != EXT2_ROOT_INO && ino < EXT2_FIRST_INO(sb))
+ return ERR_PTR(-ESTALE);
+ if (ino > le32_to_cpu(EXT2_SB(sb)->s_es->s_inodes_count))
+ return ERR_PTR(-ESTALE);
+
+ /* iget isn't really right if the inode is currently unallocated!!
+ * ext2_read_inode currently does appropriate checks, but
+ * it might be "neater" to call ext2_get_inode first and check
+ * if the inode is valid.....
+ */
+ inode = iget(sb, ino);
+ if (inode == NULL)
+ return ERR_PTR(-ENOMEM);
+ if (is_bad_inode(inode)
+ || (generation && inode->i_generation != generation)
+ ) {
+ /* we didn't find the right inode.. */
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
+ /* now to find a dentry.
+ * If possible, get a well-connected one
+ */
+ result = d_alloc_anon(inode);
+ if (!result) {
+ iput(inode);
+ return ERR_PTR(-ENOMEM);
+ }
+ return result;
+}
+
+
/* Yes, most of these are left as NULL!!
* A NULL value implies the default, which works with ext2-like file
* systems, but can be improved upon.
@@ -258,6 +298,7 @@ static struct super_operations ext2_sops
*/
static struct export_operations ext2_export_ops = {
.get_parent = ext2_get_parent,
+ .get_dentry = ext2_get_dentry,
};
static unsigned long get_sb_block(void **data)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 002 of 2] knfsd: Make ext3 reject filehandles referring to invalid inode numbers
2006-07-28 0:31 ` NeilBrown
@ 2006-07-28 0:31 ` NeilBrown
-1 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2006-07-28 0:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: nfs, linux-kernel
Inodes earlier than the 'first' inode (e.g. journal,
resize) should be rejected early - except the root inode.
Also inode numbers that are too big should be rejected early.
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/ext3/super.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff .prev/fs/ext3/super.c ./fs/ext3/super.c
--- .prev/fs/ext3/super.c 2006-07-28 10:18:55.000000000 +1000
+++ ./fs/ext3/super.c 2006-07-28 10:25:20.000000000 +1000
@@ -554,6 +554,48 @@ 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];
+ struct inode *inode;
+ struct dentry *result;
+
+ if (ino != EXT3_ROOT_INO && ino < EXT3_FIRST_INO(sb))
+ return ERR_PTR(-ESTALE);
+ if (ino > le32_to_cpu(EXT3_SB(sb)->s_es->s_inodes_count))
+ return ERR_PTR(-ESTALE);
+
+ /* iget isn't really right if the inode is currently unallocated!!
+ *
+ * ext3_read_inode will return a bad_inode if the inode had been deleted.
+ * so we should be safe.
+ *
+ * Currently we don't know the generation for parent directory, so
+ * a generation of 0 means "accept any"
+ */
+ inode = iget(sb, ino);
+ if (inode == NULL)
+ return ERR_PTR(-ENOMEM);
+ if (is_bad_inode(inode)
+ || (generation && inode->i_generation != generation)
+ ) {
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
+ /* now to find a dentry.
+ * If possible, get a well-connected one
+ */
+ result = d_alloc_anon(inode);
+ if (!result) {
+ iput(inode);
+ return ERR_PTR(-ENOMEM);
+ }
+ return result;
+}
+
#ifdef CONFIG_QUOTA
#define QTYPE2NAME(t) ((t)==USRQUOTA?"user":"group")
#define QTYPE2MOPT(on, t) ((t)==USRQUOTA?((on)##USRJQUOTA):((on)##GRPJQUOTA))
@@ -622,6 +664,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 {
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 002 of 2] knfsd: Make ext3 reject filehandles referring to invalid inode numbers
@ 2006-07-28 0:31 ` NeilBrown
0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2006-07-28 0:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: nfs, linux-kernel
Inodes earlier than the 'first' inode (e.g. journal,
resize) should be rejected early - except the root inode.
Also inode numbers that are too big should be rejected early.
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/ext3/super.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff .prev/fs/ext3/super.c ./fs/ext3/super.c
--- .prev/fs/ext3/super.c 2006-07-28 10:18:55.000000000 +1000
+++ ./fs/ext3/super.c 2006-07-28 10:25:20.000000000 +1000
@@ -554,6 +554,48 @@ 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];
+ struct inode *inode;
+ struct dentry *result;
+
+ if (ino != EXT3_ROOT_INO && ino < EXT3_FIRST_INO(sb))
+ return ERR_PTR(-ESTALE);
+ if (ino > le32_to_cpu(EXT3_SB(sb)->s_es->s_inodes_count))
+ return ERR_PTR(-ESTALE);
+
+ /* iget isn't really right if the inode is currently unallocated!!
+ *
+ * ext3_read_inode will return a bad_inode if the inode had been deleted.
+ * so we should be safe.
+ *
+ * Currently we don't know the generation for parent directory, so
+ * a generation of 0 means "accept any"
+ */
+ inode = iget(sb, ino);
+ if (inode == NULL)
+ return ERR_PTR(-ENOMEM);
+ if (is_bad_inode(inode)
+ || (generation && inode->i_generation != generation)
+ ) {
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
+ /* now to find a dentry.
+ * If possible, get a well-connected one
+ */
+ result = d_alloc_anon(inode);
+ if (!result) {
+ iput(inode);
+ return ERR_PTR(-ENOMEM);
+ }
+ return result;
+}
+
#ifdef CONFIG_QUOTA
#define QTYPE2NAME(t) ((t)==USRQUOTA?"user":"group")
#define QTYPE2MOPT(on, t) ((t)==USRQUOTA?((on)##USRJQUOTA):((on)##GRPJQUOTA))
@@ -622,6 +664,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 {
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 000 of 2] knfsd: Don't allow bad file handles to cause extX to go readonly
2006-07-28 0:31 ` NeilBrown
@ 2006-07-28 13:33 ` Christoph Hellwig
-1 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2006-07-28 13:33 UTC (permalink / raw)
To: NeilBrown; +Cc: Andrew Morton, nfs, linux-kernel
On Fri, Jul 28, 2006 at 10:31:20AM +1000, NeilBrown wrote:
> Currently, and file handle with a bad inode number in it can cause
> ext2 to go to readonly (as it looks like a corrupted filesystem)
> and could allow remote access to ext3 special files like the journal.
>
> These patches give ext2/3 their own get_dentry method which checks the
> inode number early before other bits of the code can be freaked out by
> it.
>
> These are revised versions of earlier patches. Rather than exporting
> export_iget, we open code it and simplify it slightly. This avoids
> and extra module dependancy.
This looks much better, agreed. Long-term we should switch ext2/ext2
to use iget_locked so we can propagate errors in finding the inode much
better.
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [NFS] [PATCH 000 of 2] knfsd: Don't allow bad file handles to cause extX to go readonly
@ 2006-07-28 13:33 ` Christoph Hellwig
0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2006-07-28 13:33 UTC (permalink / raw)
To: NeilBrown; +Cc: Andrew Morton, nfs, linux-kernel
On Fri, Jul 28, 2006 at 10:31:20AM +1000, NeilBrown wrote:
> Currently, and file handle with a bad inode number in it can cause
> ext2 to go to readonly (as it looks like a corrupted filesystem)
> and could allow remote access to ext3 special files like the journal.
>
> These patches give ext2/3 their own get_dentry method which checks the
> inode number early before other bits of the code can be freaked out by
> it.
>
> These are revised versions of earlier patches. Rather than exporting
> export_iget, we open code it and simplify it slightly. This avoids
> and extra module dependancy.
This looks much better, agreed. Long-term we should switch ext2/ext2
to use iget_locked so we can propagate errors in finding the inode much
better.
^ permalink raw reply [flat|nested] 8+ messages in thread