Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
> int sysfs_rename(struct sysfs_dirent *sd,
> - struct sysfs_dirent *new_parent_sd, const char *new_name)
> + struct sysfs_dirent *new_parent_sd, const void *new_ns,
> + const char *new_name)
> {
> const char *dup_name = NULL;
> int error;
> @@ -743,12 +789,12 @@ int sysfs_rename(struct sysfs_dirent *sd,
> mutex_lock(&sysfs_mutex);
>
> error = 0;
> - if ((sd->s_parent == new_parent_sd) &&
> + if ((sd->s_parent == new_parent_sd) && (sd->s_ns == new_ns) &&
> (strcmp(sd->s_name, new_name) == 0))
> goto out; /* nothing to rename */
>
> error = -EEXIST;
> - if (sysfs_find_dirent(new_parent_sd, new_name))
> + if (sysfs_find_dirent(new_parent_sd, new_ns, new_name))
> goto out;
>
> /* rename sysfs_dirent */
> @@ -770,6 +816,7 @@ int sysfs_rename(struct sysfs_dirent *sd,
> sd->s_parent = new_parent_sd;
> sysfs_link_sibling(sd);
> }
> + sd->s_ns = new_ns;
>
> error = 0;
> out:
...
> +void sysfs_exit_ns(enum kobj_ns_type type, const void *ns)
> +{
> + struct super_block *sb;
> +
> + mutex_lock(&sysfs_mutex);
> + spin_lock(&sb_lock);
> + list_for_each_entry(sb, &sysfs_fs_type.fs_supers, s_instances) {
> + struct sysfs_super_info *info = sysfs_info(sb);
> + /* Ignore superblocks that are in the process of unmounting */
> + if (sb->s_count <= S_BIAS)
> + continue;
> + /* Ignore superblocks with the wrong ns */
> + if (info->ns[type] != ns)
> + continue;
> + info->ns[type] = NULL;
> + }
> + spin_unlock(&sb_lock);
> + mutex_unlock(&sysfs_mutex);
> +}
> +
..
> @@ -136,6 +138,7 @@ int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
> const char *old, const char *new)
> {
> struct sysfs_dirent *parent_sd, *sd = NULL;
> + const void *old_ns = NULL, *new_ns = NULL;
> int result;
>
> if (!kobj)
> @@ -143,8 +146,11 @@ int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
> else
> parent_sd = kobj->sd;
>
> + if (targ->sd)
> + old_ns = targ->sd->s_ns;
> +
> result = -ENOENT;
> - sd = sysfs_get_dirent(parent_sd, old);
> + sd = sysfs_get_dirent(parent_sd, old_ns, old);
> if (!sd)
> goto out;
>
> @@ -154,7 +160,10 @@ int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
> if (sd->s_symlink.target_sd->s_dir.kobj != targ)
> goto out;
>
> - result = sysfs_rename(sd, parent_sd, new);
> + if (sysfs_ns_type(parent_sd))
> + new_ns = targ->ktype->namespace(targ);
> +
> + result = sysfs_rename(sd, parent_sd, new_ns, new);
>
> out:
> sysfs_put(sd);
This is a huge patch, and for the most part I haven't found any problems,
except potentially this one. It looks like sysfs_rename_link() checks
old_ns and new_ns before calling sysfs_rename(). But sysfs_mutex isn't
taken until sysfs_rename(). sysfs_rename() will then proceed to do
the rename, and unconditionally set sd->ns = new_ns.
In the meantime, it seems as though new_ns might have exited, and
sysfs_exit_ns() unset new_ns on the new parent dir. This means that
we'll end up with the namespace code having thought that it cleared
all new_ns's, but this file will have snuck by. Meaning an action on
the renamed file might dereference a freed namespace.
Or am I way off base?
-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
