Commit 2a109f2a4155f168047aa2f5b3a170e279bef89a

Authored by Louis Rilling
Committed by Mark Fasheh
1 parent 9a73d78cda

[PATCH] configfs: Prevent userspace from creating new entries under attaching directories

process 1: 					process 2:
configfs_mkdir("A")
  attach_group("A")
    attach_item("A")
      d_instantiate("A")
    populate_groups("A")
      mutex_lock("A")
      attach_group("A/B")
        attach_item("A")
          d_instantiate("A/B")
						mkdir("A/B/C")
						  do_path_lookup("A/B/C", LOOKUP_PARENT)
						    ok
						  lookup_create("A/B/C")
						    mutex_lock("A/B")
						    ok
						  configfs_mkdir("A/B/C")
						    ok
      attach_group("A/C")
        attach_item("A/C")
          d_instantiate("A/C")
        populate_groups("A/C")
          mutex_lock("A/C")
          attach_group("A/C/D")
            attach_item("A/C/D")
              failure
          mutex_unlock("A/C")
          detach_groups("A/C")
            nothing to do
						mkdir("A/C/E")
						  do_path_lookup("A/C/E", LOOKUP_PARENT)
						    ok
						  lookup_create("A/C/E")
						    mutex_lock("A/C")
						    ok
						  configfs_mkdir("A/C/E")
						    ok
        detach_item("A/C")
        d_delete("A/C")
      mutex_unlock("A")
      detach_groups("A")
        mutex_lock("A/B")
        detach_group("A/B")
	  detach_groups("A/B")
	    nothing since no _default_ group
          detach_item("A/B")
        mutex_unlock("A/B")
        d_delete("A/B")
    detach_item("A")
    d_delete("A")

Two bugs:

1/ "A/B/C" and "A/C/E" are created, but never removed while their parent are
removed in the end. The same could happen with symlink() instead of mkdir().

2/ "A" and "A/C" inodes are not locked while detach_item() is called on them,
   which may probably confuse VFS.

This commit fixes 1/, tagging new directories with CONFIGFS_USET_CREATING before
building the inode and instantiating the dentry, and validating the whole
group+default groups hierarchy in a second pass by clearing
CONFIGFS_USET_CREATING.
	mkdir(), symlink(), lookup(), and dir_open() simply return -ENOENT if
called in (or linking to) a directory tagged with CONFIGFS_USET_CREATING. This
does not prevent userspace from calling stat() successfuly on such directories,
but this prevents userspace from adding (children to | symlinking from/to |
read/write attributes of | listing the contents of) not validated items. In
other words, userspace will not interact with the subsystem on a new item until
the new item creation completes correctly.
	It was first proposed to re-use CONFIGFS_USET_IN_MKDIR instead of a new
flag CONFIGFS_USET_CREATING, but this generated conflicts when checking the
target of a new symlink: a valid target directory in the middle of attaching
a new user-created child item could be wrongly detected as being attached.

2/ is fixed by next commit.

Signed-off-by: Louis Rilling <louis.rilling@kerlabs.com>
Signed-off-by: Joel Becker <joel.becker@oracle.com>
Signed-off-by: Mark Fasheh <mfasheh@suse.com>

Showing 3 changed files with 103 additions and 5 deletions Side-by-side Diff

fs/configfs/configfs_internal.h
... ... @@ -49,6 +49,7 @@
49 49 #define CONFIGFS_USET_DEFAULT 0x0080
50 50 #define CONFIGFS_USET_DROPPING 0x0100
51 51 #define CONFIGFS_USET_IN_MKDIR 0x0200
  52 +#define CONFIGFS_USET_CREATING 0x0400
52 53 #define CONFIGFS_NOT_PINNED (CONFIGFS_ITEM_ATTR)
53 54  
54 55 extern struct mutex configfs_symlink_mutex;
... ... @@ -67,6 +68,7 @@
67 68 extern int configfs_create_file(struct config_item *, const struct configfs_attribute *);
68 69 extern int configfs_make_dirent(struct configfs_dirent *,
69 70 struct dentry *, void *, umode_t, int);
  71 +extern int configfs_dirent_is_ready(struct configfs_dirent *);
70 72  
71 73 extern int configfs_add_file(struct dentry *, const struct configfs_attribute *, int);
72 74 extern void configfs_hash_and_remove(struct dentry * dir, const char * name);
... ... @@ -185,7 +185,7 @@
185 185 error = configfs_dirent_exists(p->d_fsdata, d->d_name.name);
186 186 if (!error)
187 187 error = configfs_make_dirent(p->d_fsdata, d, k, mode,
188   - CONFIGFS_DIR);
  188 + CONFIGFS_DIR | CONFIGFS_USET_CREATING);
189 189 if (!error) {
190 190 error = configfs_create(d, mode, init_dir);
191 191 if (!error) {
... ... @@ -209,6 +209,9 @@
209 209 * configfs_create_dir - create a directory for an config_item.
210 210 * @item: config_itemwe're creating directory for.
211 211 * @dentry: config_item's dentry.
  212 + *
  213 + * Note: user-created entries won't be allowed under this new directory
  214 + * until it is validated by configfs_dir_set_ready()
212 215 */
213 216  
214 217 static int configfs_create_dir(struct config_item * item, struct dentry *dentry)
... ... @@ -231,6 +234,44 @@
231 234 return error;
232 235 }
233 236  
  237 +/*
  238 + * Allow userspace to create new entries under a new directory created with
  239 + * configfs_create_dir(), and under all of its chidlren directories recursively.
  240 + * @sd configfs_dirent of the new directory to validate
  241 + *
  242 + * Caller must hold configfs_dirent_lock.
  243 + */
  244 +static void configfs_dir_set_ready(struct configfs_dirent *sd)
  245 +{
  246 + struct configfs_dirent *child_sd;
  247 +
  248 + sd->s_type &= ~CONFIGFS_USET_CREATING;
  249 + list_for_each_entry(child_sd, &sd->s_children, s_sibling)
  250 + if (child_sd->s_type & CONFIGFS_USET_CREATING)
  251 + configfs_dir_set_ready(child_sd);
  252 +}
  253 +
  254 +/*
  255 + * Check that a directory does not belong to a directory hierarchy being
  256 + * attached and not validated yet.
  257 + * @sd configfs_dirent of the directory to check
  258 + *
  259 + * @return non-zero iff the directory was validated
  260 + *
  261 + * Note: takes configfs_dirent_lock, so the result may change from false to true
  262 + * in two consecutive calls, but never from true to false.
  263 + */
  264 +int configfs_dirent_is_ready(struct configfs_dirent *sd)
  265 +{
  266 + int ret;
  267 +
  268 + spin_lock(&configfs_dirent_lock);
  269 + ret = !(sd->s_type & CONFIGFS_USET_CREATING);
  270 + spin_unlock(&configfs_dirent_lock);
  271 +
  272 + return ret;
  273 +}
  274 +
234 275 int configfs_create_link(struct configfs_symlink *sl,
235 276 struct dentry *parent,
236 277 struct dentry *dentry)
237 278  
... ... @@ -330,8 +371,20 @@
330 371 struct configfs_dirent * parent_sd = dentry->d_parent->d_fsdata;
331 372 struct configfs_dirent * sd;
332 373 int found = 0;
333   - int err = 0;
  374 + int err;
334 375  
  376 + /*
  377 + * Fake invisibility if dir belongs to a group/default groups hierarchy
  378 + * being attached
  379 + *
  380 + * This forbids userspace to read/write attributes of items which may
  381 + * not complete their initialization, since the dentries of the
  382 + * attributes won't be instantiated.
  383 + */
  384 + err = -ENOENT;
  385 + if (!configfs_dirent_is_ready(parent_sd))
  386 + goto out;
  387 +
335 388 list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
336 389 if (sd->s_type & CONFIGFS_NOT_PINNED) {
337 390 const unsigned char * name = configfs_get_name(sd);
... ... @@ -353,6 +406,7 @@
353 406 return simple_lookup(dir, dentry, nd);
354 407 }
355 408  
  409 +out:
356 410 return ERR_PTR(err);
357 411 }
358 412  
... ... @@ -1044,6 +1098,16 @@
1044 1098 }
1045 1099  
1046 1100 sd = dentry->d_parent->d_fsdata;
  1101 +
  1102 + /*
  1103 + * Fake invisibility if dir belongs to a group/default groups hierarchy
  1104 + * being attached
  1105 + */
  1106 + if (!configfs_dirent_is_ready(sd)) {
  1107 + ret = -ENOENT;
  1108 + goto out;
  1109 + }
  1110 +
1047 1111 if (!(sd->s_type & CONFIGFS_USET_DIR)) {
1048 1112 ret = -EPERM;
1049 1113 goto out;
... ... @@ -1142,6 +1206,8 @@
1142 1206  
1143 1207 spin_lock(&configfs_dirent_lock);
1144 1208 sd->s_type &= ~CONFIGFS_USET_IN_MKDIR;
  1209 + if (!ret)
  1210 + configfs_dir_set_ready(dentry->d_fsdata);
1145 1211 spin_unlock(&configfs_dirent_lock);
1146 1212  
1147 1213 out_unlink:
1148 1214  
1149 1215  
... ... @@ -1322,13 +1388,24 @@
1322 1388 {
1323 1389 struct dentry * dentry = file->f_path.dentry;
1324 1390 struct configfs_dirent * parent_sd = dentry->d_fsdata;
  1391 + int err;
1325 1392  
1326 1393 mutex_lock(&dentry->d_inode->i_mutex);
1327   - file->private_data = configfs_new_dirent(parent_sd, NULL);
  1394 + /*
  1395 + * Fake invisibility if dir belongs to a group/default groups hierarchy
  1396 + * being attached
  1397 + */
  1398 + err = -ENOENT;
  1399 + if (configfs_dirent_is_ready(parent_sd)) {
  1400 + file->private_data = configfs_new_dirent(parent_sd, NULL);
  1401 + if (IS_ERR(file->private_data))
  1402 + err = PTR_ERR(file->private_data);
  1403 + else
  1404 + err = 0;
  1405 + }
1328 1406 mutex_unlock(&dentry->d_inode->i_mutex);
1329 1407  
1330   - return IS_ERR(file->private_data) ? PTR_ERR(file->private_data) : 0;
1331   -
  1408 + return err;
1332 1409 }
1333 1410  
1334 1411 static int configfs_dir_close(struct inode *inode, struct file *file)
... ... @@ -1499,6 +1576,10 @@
1499 1576 if (err) {
1500 1577 d_delete(dentry);
1501 1578 dput(dentry);
  1579 + } else {
  1580 + spin_lock(&configfs_dirent_lock);
  1581 + configfs_dir_set_ready(dentry->d_fsdata);
  1582 + spin_unlock(&configfs_dirent_lock);
1502 1583 }
1503 1584 }
1504 1585  
fs/configfs/symlink.c
... ... @@ -76,6 +76,9 @@
76 76 struct configfs_symlink *sl;
77 77 int ret;
78 78  
  79 + ret = -ENOENT;
  80 + if (!configfs_dirent_is_ready(target_sd))
  81 + goto out;
79 82 ret = -ENOMEM;
80 83 sl = kmalloc(sizeof(struct configfs_symlink), GFP_KERNEL);
81 84 if (sl) {
... ... @@ -100,6 +103,7 @@
100 103 }
101 104 }
102 105  
  106 +out:
103 107 return ret;
104 108 }
105 109  
... ... @@ -129,6 +133,7 @@
129 133 {
130 134 int ret;
131 135 struct nameidata nd;
  136 + struct configfs_dirent *sd;
132 137 struct config_item *parent_item;
133 138 struct config_item *target_item;
134 139 struct config_item_type *type;
135 140  
... ... @@ -137,9 +142,19 @@
137 142 if (dentry->d_parent == configfs_sb->s_root)
138 143 goto out;
139 144  
  145 + sd = dentry->d_parent->d_fsdata;
  146 + /*
  147 + * Fake invisibility if dir belongs to a group/default groups hierarchy
  148 + * being attached
  149 + */
  150 + ret = -ENOENT;
  151 + if (!configfs_dirent_is_ready(sd))
  152 + goto out;
  153 +
140 154 parent_item = configfs_get_config_item(dentry->d_parent);
141 155 type = parent_item->ci_type;
142 156  
  157 + ret = -EPERM;
143 158 if (!type || !type->ct_item_ops ||
144 159 !type->ct_item_ops->allow_link)
145 160 goto out_put;