Re: [PATCH 2/3] ensure unique i_ino in filesystems without permanent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff Layton wrote:
[email protected] wrote:
>> Doh! Thanks for explaining that. Here's a respun patch with your suggestion >> incorporated. Seems to build correctly without stdbool.h. In fact, I don't see >> a stdbool.h in Linus' tree as of this morning. Are you sure that it's needed?
>
> include/linux/types.h:36:typedef _Bool                   bool;
>
> It's already in there.  And <linux/stddef.h> has false and true.
>
> Sorry; I write more user-level code.
>
> The'd be built-in, except that C99 didn't want to add a kewords that
> might conflict with existing code.

Ok, that makes sense. Though I just found a bug that I missed. I forgot to change the
EXPORT_SYMBOL in libfs.c. This should fix it:

Signed-off-by: Jeff Layton <[email protected]>

--- linux-2.6/fs/debugfs/inode.c.super
+++ linux-2.6/fs/debugfs/inode.c
@@ -107,7 +107,7 @@ static int debug_fill_super(struct super
 {
     static struct tree_descr debug_files[] = {{""}};

-    return simple_fill_super(sb, DEBUGFS_MAGIC, debug_files);
+    return registered_fill_super(sb, DEBUGFS_MAGIC, debug_files);
 }

 static int debug_get_sb(struct file_system_type *fs_type,
--- linux-2.6/fs/fuse/control.c.super
+++ linux-2.6/fs/fuse/control.c
@@ -163,7 +163,7 @@ static int fuse_ctl_fill_super(struct su
     struct fuse_conn *fc;
     int err;

-    err = simple_fill_super(sb, FUSE_CTL_SUPER_MAGIC, &empty_descr);
+    err = registered_fill_super(sb, FUSE_CTL_SUPER_MAGIC, &empty_descr);
     if (err)
         return err;

--- linux-2.6/fs/libfs.c.super
+++ linux-2.6/fs/libfs.c
@@ -215,7 +215,7 @@ int get_sb_pseudo(struct file_system_typ
     s->s_op = ops ? ops : &default_ops;
     s->s_time_gran = 1;
     root = new_inode(s);
-    if (!root)
+    if (!root || iunique_register(root, 0))
         goto Enomem;
     root->i_mode = S_IFDIR | S_IRUSR | S_IWUSR;
     root->i_uid = root->i_gid = 0;
@@ -356,7 +356,8 @@ int simple_commit_write(struct file *fil
     return 0;
 }


If iunique_register() fails, does not this create a memory leak
because root will need to get iput()'d?

-int simple_fill_super(struct super_block *s, int magic, struct tree_descr *files)
+int __simple_fill_super(struct super_block *s, int magic,
+            struct tree_descr *files, bool registered)
 {
     static struct super_operations s_ops = {.statfs = simple_statfs};
     struct inode *inode;
@@ -380,6 +381,12 @@ int simple_fill_super(struct super_block
     inode->i_op = &simple_dir_inode_operations;
     inode->i_fop = &simple_dir_operations;
     inode->i_nlink = 2;
+    /*
+     * set this as high as a 32 bit val as possible to avoid collisions.
+     * This is also well above the highest value that iunique_register
+         * will assign to an inode
+     */
+    inode->i_ino = 0xffffffff;
     root = d_alloc_root(inode);
     if (!root) {
         iput(inode);
@@ -394,12 +401,15 @@ int simple_fill_super(struct super_block
         inode = new_inode(s);
         if (!inode)
             goto out;
+        if (!registered)
+            inode->i_ino = i;
+        else if (iunique_register(inode, 0))
+            goto out;

Does this not create a memory leak as well?

   Thanx...

      ps

         inode->i_mode = S_IFREG | files->mode;
         inode->i_uid = inode->i_gid = 0;
         inode->i_blocks = 0;
         inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
         inode->i_fop = files->ops;
-        inode->i_ino = i;
         d_add(dentry, inode);
     }
     s->s_root = root;
@@ -618,7 +628,7 @@ EXPORT_SYMBOL(simple_dir_inode_operation
 EXPORT_SYMBOL(simple_dir_operations);
 EXPORT_SYMBOL(simple_empty);
 EXPORT_SYMBOL(d_alloc_name);
-EXPORT_SYMBOL(simple_fill_super);
+EXPORT_SYMBOL(__simple_fill_super);
 EXPORT_SYMBOL(simple_getattr);
 EXPORT_SYMBOL(simple_link);
 EXPORT_SYMBOL(simple_lookup);
--- linux-2.6/include/linux/fs.h.super
+++ linux-2.6/include/linux/fs.h
@@ -1879,10 +1879,35 @@ extern const struct file_operations simp
 extern struct inode_operations simple_dir_inode_operations;
struct tree_descr { char *name; const struct file_operations *ops; int mode; };
 struct dentry *d_alloc_name(struct dentry *, const char *);
-extern int simple_fill_super(struct super_block *, int, struct tree_descr *);
+extern int __simple_fill_super(struct super_block *s, int magic,
+                struct tree_descr *files, bool registered);
extern int simple_pin_fs(struct file_system_type *, struct vfsmount **mount, int *count);
 extern void simple_release_fs(struct vfsmount **mount, int *count);

+/*
+ * Fill a superblock with a standard set of fields, and add the entries in the + * "files" struct. Assign i_ino values to the files sequentially. This function + * is appropriate for filesystems that need a particular i_ino value assigned
+ * to a particular "files" entry.
+ */
+static inline int simple_fill_super(struct super_block *s, int magic,
+                    struct tree_descr *files)
+{
+    return __simple_fill_super(s, magic, files, false);
+}
+
+/*
+ * Just like simple_fill_super, but does an iunique_register on the inodes + * created for "files" entries. This function is appropriate when you don't + * need a particular i_ino value assigned to each files entry, and when the
+ * filesystem will have other registered inodes.
+ */
+static inline int registered_fill_super(struct super_block *s, int magic,
+                        struct tree_descr *files)
+{
+    return __simple_fill_super(s, magic, files, true);
+}
+
extern ssize_t simple_read_from_buffer(void __user *, size_t, loff_t *, const void *, size_t);

 #ifdef CONFIG_MIGRATION
--- linux-2.6/security/inode.c.super
+++ linux-2.6/security/inode.c
@@ -130,7 +130,7 @@ static int fill_super(struct super_block
 {
     static struct tree_descr files[] = {{""}};

-    return simple_fill_super(sb, SECURITYFS_MAGIC, files);
+    return registered_fill_super(sb, SECURITYFS_MAGIC, files);
 }

 static int get_sb(struct file_system_type *fs_type,
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux