LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
@ 2007-01-11 20:15 Jeff Layton
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2007-01-11 20:15 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel

Amateur mistake. Fix the if statement that I just added. That'll teach me to
not test these things.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/drivers/infiniband/hw/ipath/ipath_fs.c b/drivers/infiniband/hw/ipath/ipath_fs.c
index 79a60f0..6ce7926 100644
--- a/drivers/infiniband/hw/ipath/ipath_fs.c
+++ b/drivers/infiniband/hw/ipath/ipath_fs.c
@@ -510,7 +510,7 @@ static int ipathfs_fill_super(struct super_block *sb, void *data,
 	int ret;
 
 	static struct tree_descr files[] = {
-		[1] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
+		[2] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
 		{""},
 	};
 
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index c2e0825..023d825 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -727,8 +727,8 @@ static struct super_operations s_ops = {
 static int bm_fill_super(struct super_block * sb, void * data, int silent)
 {
 	static struct tree_descr bm_files[] = {
-		[1] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
-		[2] = {"register", &bm_register_operations, S_IWUSR},
+		[2] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
+		[3] = {"register", &bm_register_operations, S_IWUSR},
 		/* last one */ {""}
 	};
 	int err = simple_fill_super(sb, 0x42494e4d, bm_files);
diff --git a/fs/inode.c b/fs/inode.c
diff --git a/fs/libfs.c b/fs/libfs.c
index 503898d..9b77f89 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -217,6 +217,12 @@ int get_sb_pseudo(struct file_system_type *fs_type, char *name,
 	root = new_inode(s);
 	if (!root)
 		goto Enomem;
+	/*
+	 * since this is the first inode, make it number 1. New inodes created
+	 * after this must take care not to collide with it (by passing
+	 * max_reserved of 1 to iunique).
+	 */
+	root->i_ino = 1;
 	root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR;
 	root->i_uid = root->i_gid = 0;
 	root->i_atime = root->i_mtime = root->i_ctime = CURRENT_TIME;
@@ -356,6 +362,11 @@ int simple_commit_write(struct file *file, struct page *page,
 	return 0;
 }
 
+/*
+ * the inodes created here are not hashed. If you use iunique to generate
+ * unique inode values later for this filesystem, then you must take care
+ * to pass it an appropriate max_reserved value to avoid collisions.
+ */
 int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files)
 {
 	static struct super_operations s_ops = {.statfs = simple_statfs};
@@ -373,6 +384,11 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	inode = new_inode(s);
 	if (!inode)
 		return -ENOMEM;
+	/*
+	 * because the root inode is 1, the files array must not contain an
+	 * entry at index 1
+	 */
+	inode->i_ino = 1;
 	inode->i_mode = S_IFDIR | 0755;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_blocks = 0;
@@ -388,6 +404,13 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	for (i = 0; !files->name || files->name[0]; i++, files++) {
 		if (!files->name)
 			continue;
+
+		/* warn if it tries to conflict with the root inode */
+		if (unlikely(i == 1))
+			printk(KERN_WARNING "%s: %s passed in a files array"
+				"with an index of 1!\n", __func__,
+				s->s_type->name);
+
 		dentry = d_alloc_name(root, files->name);
 		if (!dentry)
 			goto out;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
@ 2007-01-16 18:57 Jeff Layton
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2007-01-16 18:57 UTC (permalink / raw)
  To: apkm; +Cc: linux-fsdevel, linux-kernel

This patch makes it so that simple_fill_super and get_sb_pseudo assign their
root inodes to be number 1. It also fixes up a couple of callers of
simple_fill_super that were passing in files arrays that had an index at
number 1, and adds a warning for any caller that sends in such an array.

It would have been nice to have made it so that it wasn't possible to make
such a collision, but some callers need to be able to control what inode
number their entries get, so I think this is the best that can be done.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/drivers/infiniband/hw/ipath/ipath_fs.c b/drivers/infiniband/hw/ipath/ipath_fs.c
index 79a60f0..6ce7926 100644
--- a/drivers/infiniband/hw/ipath/ipath_fs.c
+++ b/drivers/infiniband/hw/ipath/ipath_fs.c
@@ -510,7 +510,7 @@ static int ipathfs_fill_super(struct super_block *sb, void *data,
 	int ret;
 
 	static struct tree_descr files[] = {
-		[1] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
+		[2] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
 		{""},
 	};
 
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index c2e0825..023d825 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -727,8 +727,8 @@ static struct super_operations s_ops = {
 static int bm_fill_super(struct super_block * sb, void * data, int silent)
 {
 	static struct tree_descr bm_files[] = {
-		[1] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
-		[2] = {"register", &bm_register_operations, S_IWUSR},
+		[2] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
+		[3] = {"register", &bm_register_operations, S_IWUSR},
 		/* last one */ {""}
 	};
 	int err = simple_fill_super(sb, 0x42494e4d, bm_files);
diff --git a/fs/inode.c b/fs/inode.c
diff --git a/fs/libfs.c b/fs/libfs.c
index 503898d..9b77f89 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -217,6 +217,12 @@ int get_sb_pseudo(struct file_system_type *fs_type, char *name,
 	root = new_inode(s);
 	if (!root)
 		goto Enomem;
+	/*
+	 * since this is the first inode, make it number 1. New inodes created
+	 * after this must take care not to collide with it (by passing
+	 * max_reserved of 1 to iunique).
+	 */
+	root->i_ino = 1;
 	root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR;
 	root->i_uid = root->i_gid = 0;
 	root->i_atime = root->i_mtime = root->i_ctime = CURRENT_TIME;
@@ -356,6 +362,11 @@ int simple_commit_write(struct file *file, struct page *page,
 	return 0;
 }
 
+/*
+ * the inodes created here are not hashed. If you use iunique to generate
+ * unique inode values later for this filesystem, then you must take care
+ * to pass it an appropriate max_reserved value to avoid collisions.
+ */
 int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files)
 {
 	static struct super_operations s_ops = {.statfs = simple_statfs};
@@ -373,6 +384,11 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	inode = new_inode(s);
 	if (!inode)
 		return -ENOMEM;
+	/*
+	 * because the root inode is 1, the files array must not contain an
+	 * entry at index 1
+	 */
+	inode->i_ino = 1;
 	inode->i_mode = S_IFDIR | 0755;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_blocks = 0;
@@ -388,6 +404,13 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	for (i = 0; !files->name || files->name[0]; i++, files++) {
 		if (!files->name)
 			continue;
+
+		/* warn if it tries to conflict with the root inode */
+		if (unlikely(i == 1))
+			printk(KERN_WARNING "%s: %s passed in a files array"
+				"with an index of 1!\n", __func__,
+				s->s_type->name);
+
 		dentry = d_alloc_name(root, files->name);
 		if (!dentry)
 			goto out;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
@ 2007-01-11 20:08 Jeff Layton
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2007-01-11 20:08 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel

Yet another respin. It's awfully easy to pass in a files array that will
conflict with the root inode. Add in a check and warning for that. It would
be nice if it were possible to make it so this couldn't happen by design,
I don't see how to do that, given that some filesystems need control over
what inode numbers these entries receive.

This one is untested, but the only change from the last one is the
if/printk.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/drivers/infiniband/hw/ipath/ipath_fs.c b/drivers/infiniband/hw/ipath/ipath_fs.c
index 79a60f0..6ce7926 100644
--- a/drivers/infiniband/hw/ipath/ipath_fs.c
+++ b/drivers/infiniband/hw/ipath/ipath_fs.c
@@ -510,7 +510,7 @@ static int ipathfs_fill_super(struct super_block *sb, void *data,
 	int ret;
 
 	static struct tree_descr files[] = {
-		[1] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
+		[2] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
 		{""},
 	};
 
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index c2e0825..023d825 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -727,8 +727,8 @@ static struct super_operations s_ops = {
 static int bm_fill_super(struct super_block * sb, void * data, int silent)
 {
 	static struct tree_descr bm_files[] = {
-		[1] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
-		[2] = {"register", &bm_register_operations, S_IWUSR},
+		[2] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
+		[3] = {"register", &bm_register_operations, S_IWUSR},
 		/* last one */ {""}
 	};
 	int err = simple_fill_super(sb, 0x42494e4d, bm_files);
diff --git a/fs/inode.c b/fs/inode.c
diff --git a/fs/libfs.c b/fs/libfs.c
index 503898d..c8f5532 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -217,6 +217,12 @@ int get_sb_pseudo(struct file_system_type *fs_type, char *name,
 	root = new_inode(s);
 	if (!root)
 		goto Enomem;
+	/*
+	 * since this is the first inode, make it number 1. New inodes created
+	 * after this must take care not to collide with it (by passing
+	 * max_reserved of 1 to iunique).
+	 */
+	root->i_ino = 1;
 	root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR;
 	root->i_uid = root->i_gid = 0;
 	root->i_atime = root->i_mtime = root->i_ctime = CURRENT_TIME;
@@ -356,6 +362,11 @@ int simple_commit_write(struct file *file, struct page *page,
 	return 0;
 }
 
+/*
+ * the inodes created here are not hashed. If you use iunique to generate
+ * unique inode values later for this filesystem, then you must take care
+ * to pass it an appropriate max_reserved value to avoid collisions.
+ */
 int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files)
 {
 	static struct super_operations s_ops = {.statfs = simple_statfs};
@@ -373,6 +384,11 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	inode = new_inode(s);
 	if (!inode)
 		return -ENOMEM;
+	/*
+	 * because the root inode is 1, the files array must not contain an
+	 * entry at index 1
+	 */
+	inode->i_ino = 1;
 	inode->i_mode = S_IFDIR | 0755;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_blocks = 0;
@@ -388,6 +404,13 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	for (i = 0; !files->name || files->name[0]; i++, files++) {
 		if (!files->name)
 			continue;
+
+		/* warn if it tries to conflict with the root inode */
+		if (unlikely(i = 1))
+			printk(KERN_WARNING "%s: %s passed in a files array"
+				"with an index of 1!\n", __func__,
+				s->s_type->name);
+
 		dentry = d_alloc_name(root, files->name);
 		if (!dentry)
 			goto out;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
@ 2007-01-11 19:43 Jeff Layton
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2007-01-11 19:43 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel

Here is a respun patch, which makes the root inode created by simple_fill_super
be number 1. This also adds some comments about appropriate usage of iunique
with this function, and caution about not passing it a files array with an
entry at index 1.

A couple of filesystems were passing in files structs with an entry at
index 1. That would have created a conflict, so I changed them so that
there would be not be.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/drivers/infiniband/hw/ipath/ipath_fs.c b/drivers/infiniband/hw/ipath/ipath_fs.c
index 79a60f0..6ce7926 100644
--- a/drivers/infiniband/hw/ipath/ipath_fs.c
+++ b/drivers/infiniband/hw/ipath/ipath_fs.c
@@ -510,7 +510,7 @@ static int ipathfs_fill_super(struct super_block *sb, void *data,
 	int ret;
 
 	static struct tree_descr files[] = {
-		[1] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
+		[2] = {"atomic_stats", &atomic_stats_ops, S_IRUGO},
 		{""},
 	};
 
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index c2e0825..023d825 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -727,8 +727,8 @@ static struct super_operations s_ops = {
 static int bm_fill_super(struct super_block * sb, void * data, int silent)
 {
 	static struct tree_descr bm_files[] = {
-		[1] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
-		[2] = {"register", &bm_register_operations, S_IWUSR},
+		[2] = {"status", &bm_status_operations, S_IWUSR|S_IRUGO},
+		[3] = {"register", &bm_register_operations, S_IWUSR},
 		/* last one */ {""}
 	};
 	int err = simple_fill_super(sb, 0x42494e4d, bm_files);
diff --git a/fs/inode.c b/fs/inode.c
diff --git a/fs/libfs.c b/fs/libfs.c
index 503898d..d9a299b 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -217,6 +217,12 @@ int get_sb_pseudo(struct file_system_type *fs_type, char *name,
 	root = new_inode(s);
 	if (!root)
 		goto Enomem;
+	/*
+	 * since this is the first inode, make it number 1. New inodes created
+	 * after this must take care not to collide with it (by passing
+	 * max_reserved of 1 to iunique).
+	 */
+	root->i_ino = 1;
 	root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR;
 	root->i_uid = root->i_gid = 0;
 	root->i_atime = root->i_mtime = root->i_ctime = CURRENT_TIME;
@@ -356,6 +362,11 @@ int simple_commit_write(struct file *file, struct page *page,
 	return 0;
 }
 
+/*
+ * the inodes created here are not hashed. If you use iunique to generate
+ * unique inode values later for this filesystem, then you must take care
+ * to pass it an appropriate max_reserved value to avoid collisions.
+ */
 int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files)
 {
 	static struct super_operations s_ops = {.statfs = simple_statfs};
@@ -373,6 +384,11 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	inode = new_inode(s);
 	if (!inode)
 		return -ENOMEM;
+	/*
+	 * because the root inode is 1, the files array must not contain an
+	 * entry at index 1
+	 */
+	inode->i_ino = 1;
 	inode->i_mode = S_IFDIR | 0755;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_blocks = 0;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
  2007-01-10 21:51   ` Eric Sandeen
@ 2007-01-11 16:46     ` Jeff Layton
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2007-01-11 16:46 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Christoph Hellwig, Jeff Layton, linux-fsdevel, linux-kernel,
	esandeen, aviro

Eric Sandeen wrote:
 > Christoph Hellwig wrote:
 >
 >>>  		return -ENOMEM;
 >>> +	/* set to high value to try and avoid collisions with loop below */
 >>> +	inode->i_ino = 0xffffffff;
 >>> +	insert_inode_hash(inode);
 >> This is odd.  Can't we just add some constant base to the loop below?
 >>
 > I thought the same thing, but Jeff pointed out that
 > nfsctl_transaction_write does ops based on inode numbers, and they maybe
 > can't move around?
 >
 >         rv =  write_op[ino](file, data, size);
 >
 > However, nfsd's call to simple_fill_super already sends in a set of
 > files starting at inode 2:
 >
 > enum {
 >         NFSD_Root = 1,
 >         NFSD_Svc,
 > ...
 >
 >         static struct tree_descr nfsd_files[] = {
 >                 [NFSD_Svc] = {".svc", &transaction_ops, S_IWUSR},
 > ...
 >         return simple_fill_super(sb, 0x6e667364, nfsd_files);
 >
 > which does...
 >
 >       for (i = 0; !files->name || files->name[0]; i++, files++) {
 >                 if (!files->name)
 >                         continue;
 > ...
 >                 inode->i_ino = i;
 >
 > So I think it's already expecting the root inode at one, and the other
 > files starting at 2?
 >

Very interesting -- I saw that nfsctl depended on certain values for i_ino,
but I didn't notice that it had reserved the first slot for NFSD_Root. The
only other caller of simple_fill_super that seems to depend on the inode
numbers is selinuxfs, and that one has this:

enum sel_inos {
         SEL_ROOT_INO = 2,

...so it looks like we can probably use 1 for the root inode (though we might
want to fix selinuxfs to set SEL_ROOT_INO = 1). I'll have a closer look to be
sure that this is all correct and respin this patch since that seems like it
would be less magic-number-y.

Thanks,
Jeff

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
  2007-01-10 21:22 ` Christoph Hellwig
@ 2007-01-10 21:51   ` Eric Sandeen
  2007-01-11 16:46     ` Jeff Layton
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2007-01-10 21:51 UTC (permalink / raw)
  To: Christoph Hellwig, Jeff Layton, linux-fsdevel, linux-kernel,
	esandeen, aviro

Christoph Hellwig wrote:

> 
>>  		return -ENOMEM;
>> +	/* set to high value to try and avoid collisions with loop below */
>> +	inode->i_ino = 0xffffffff;
>> +	insert_inode_hash(inode);
> 
> This is odd.  Can't we just add some constant base to the loop below?
>
I thought the same thing, but Jeff pointed out that
nfsctl_transaction_write does ops based on inode numbers, and they maybe
can't move around?

        rv =  write_op[ino](file, data, size);

However, nfsd's call to simple_fill_super already sends in a set of
files starting at inode 2:

enum {
        NFSD_Root = 1,
        NFSD_Svc,
...

        static struct tree_descr nfsd_files[] = {
                [NFSD_Svc] = {".svc", &transaction_ops, S_IWUSR},
...
        return simple_fill_super(sb, 0x6e667364, nfsd_files);

which does...

      for (i = 0; !files->name || files->name[0]; i++, files++) {
                if (!files->name)
                        continue;
...
                inode->i_ino = i;

So I think it's already expecting the root inode at one, and the other
files starting at 2?

-Eric

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
  2007-01-08 20:47 Jeff Layton
@ 2007-01-10 21:22 ` Christoph Hellwig
  2007-01-10 21:51   ` Eric Sandeen
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2007-01-10 21:22 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-fsdevel, linux-kernel, esandeen, aviro

On Mon, Jan 08, 2007 at 03:47:13PM -0500, Jeff Layton wrote:
> This changes the superblock creation routines that call new_inode to take steps
> to avoid later collisions with other inodes that get created. I took the
> approach here of not hashing things unless is was strictly necessary, though
> that does mean that filesystem authors need to be careful to avoid collisions
> by calling iunique properly.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 503898d..5bdaf00 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -217,6 +217,12 @@ int get_sb_pseudo(struct file_system_type *fs_type, char *name,
>  	root = new_inode(s);
>  	if (!root)
>  		goto Enomem;
> +	/*
> +	 * since this is the first inode, make it number 1. New inodes created
> +         * after this must take care not to collide with it (by passing
> +	 * max_reserved of 1 to iunique).
> +	 */
> +	root->i_ino = 1;

Ok.

>  		return -ENOMEM;
> +	/* set to high value to try and avoid collisions with loop below */
> +	inode->i_ino = 0xffffffff;
> +	insert_inode_hash(inode);

This is odd.  Can't we just add some constant base to the loop below?

>  	inode->i_mode = S_IFDIR | 0755;
>  	inode->i_uid = inode->i_gid = 0;
>  	inode->i_blocks = 0;
> @@ -399,6 +408,11 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
>  		inode->i_blocks = 0;
>  		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
>  		inode->i_fop = files->ops;
> +		/*
> +		 * no need to hash these, but you need to make sure that any
> +		 * calls to iunique on this mount call it with a max_reserved
> +		 * value high enough to avoid collisions with these inodes.
> +		 */
>  		inode->i_ino = i;
>  		d_add(dentry, inode);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
@ 2007-01-08 20:47 Jeff Layton
  2007-01-10 21:22 ` Christoph Hellwig
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2007-01-08 20:47 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel; +Cc: esandeen, aviro

This changes the superblock creation routines that call new_inode to take steps
to avoid later collisions with other inodes that get created. I took the
approach here of not hashing things unless is was strictly necessary, though
that does mean that filesystem authors need to be careful to avoid collisions
by calling iunique properly.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/fs/libfs.c b/fs/libfs.c
index 503898d..5bdaf00 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -217,6 +217,12 @@ int get_sb_pseudo(struct file_system_type *fs_type, char *name,
 	root = new_inode(s);
 	if (!root)
 		goto Enomem;
+	/*
+	 * since this is the first inode, make it number 1. New inodes created
+         * after this must take care not to collide with it (by passing
+	 * max_reserved of 1 to iunique).
+	 */
+	root->i_ino = 1;
 	root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR;
 	root->i_uid = root->i_gid = 0;
 	root->i_atime = root->i_mtime = root->i_ctime = CURRENT_TIME;
@@ -373,6 +379,9 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	inode = new_inode(s);
 	if (!inode)
 		return -ENOMEM;
+	/* set to high value to try and avoid collisions with loop below */
+	inode->i_ino = 0xffffffff;
+	insert_inode_hash(inode);
 	inode->i_mode = S_IFDIR | 0755;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_blocks = 0;
@@ -399,6 +408,11 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 		inode->i_blocks = 0;
 		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 		inode->i_fop = files->ops;
+		/*
+		 * no need to hash these, but you need to make sure that any
+		 * calls to iunique on this mount call it with a max_reserved
+		 * value high enough to avoid collisions with these inodes.
+		 */
 		inode->i_ino = i;
 		d_add(dentry, inode);
 	}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes
@ 2006-12-29 19:11 Jeff Layton
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2006-12-29 19:11 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel

This changes the superblock creation routines that call new_inode to take steps
to avoid later collisions with other inodes that get created. I took the
approach here of not hashing things unless is was strictly necessary, though
that does mean that filesystem authors need to be careful to avoid collisions
by calling iunique properly.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/fs/libfs.c b/fs/libfs.c
index 503898d..5bdaf00 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -217,6 +217,12 @@ int get_sb_pseudo(struct file_system_type *fs_type, char *name,
 	root = new_inode(s);
 	if (!root)
 		goto Enomem;
+	/*
+	 * since this is the first inode, make it number 1. New inodes created
+         * after this must take care not to collide with it (by passing
+	 * max_reserved of 1 to iunique).
+	 */
+	root->i_ino = 1;
 	root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR;
 	root->i_uid = root->i_gid = 0;
 	root->i_atime = root->i_mtime = root->i_ctime = CURRENT_TIME;
@@ -373,6 +379,9 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 	inode = new_inode(s);
 	if (!inode)
 		return -ENOMEM;
+	/* set to high value to try and avoid collisions with loop below */
+	inode->i_ino = 0xffffffff;
+	insert_inode_hash(inode);
 	inode->i_mode = S_IFDIR | 0755;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_blocks = 0;
@@ -399,6 +408,11 @@ int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files
 		inode->i_blocks = 0;
 		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 		inode->i_fop = files->ops;
+		/*
+		 * no need to hash these, but you need to make sure that any
+		 * calls to iunique on this mount call it with a max_reserved
+		 * value high enough to avoid collisions with these inodes.
+		 */
 		inode->i_ino = i;
 		d_add(dentry, inode);
 	}

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2007-01-16 18:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-11 20:15 [PATCH 2/3] change libfs sb creation routines to avoid collisions with their root inodes Jeff Layton
  -- strict thread matches above, loose matches on Subject: below --
2007-01-16 18:57 Jeff Layton
2007-01-11 20:08 Jeff Layton
2007-01-11 19:43 Jeff Layton
2007-01-08 20:47 Jeff Layton
2007-01-10 21:22 ` Christoph Hellwig
2007-01-10 21:51   ` Eric Sandeen
2007-01-11 16:46     ` Jeff Layton
2006-12-29 19:11 Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).