• Louis Rilling's avatar
    [PATCH] configfs: Prevent userspace from creating new entries under attaching directories · 2a109f2a
    Louis Rilling authored
    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: default avatarLouis Rilling <louis.rilling@kerlabs.com>
    Signed-off-by: default avatarJoel Becker <joel.becker@oracle.com>
    Signed-off-by: default avatarMark Fasheh <mfasheh@suse.com>
    2a109f2a
configfs_internal.h 4.92 KB