On Thu, Apr 02, 2015 at 08:56:23PM -0500, Eric W. Biederman wrote:
> +static void mark_violated_mounts(struct dentry *dentry, struct dentry *target)
> +{
> + /* Mark all mountroots that are ancestors of dentry
> + * that do not share a common ancestor with target
> + *
> + * This function assumes both dentries are part of a DAG.
> + */
> + struct dentry *p;
> +
> + for (p = dentry->d_parent; !IS_ROOT(p); p = p->d_parent) {
> + if (!d_mountroot(p))
> + continue;
> +
> + if (d_ancestor(p, target))
> + break;
Egads... You do realize that you'll keep walking the path from target to
root again and again? It's a tree and we have already walked up to the root.
So we know the depths and tree topology can't change due to the rename_lock
being held by caller.
> + if (!IS_ROOT(dentry) && !IS_ROOT(target)) {
> + mark_violated_mounts(dentry, target);
> + mark_violated_mounts(target, dentry);
> + }
> +
> dentry_lock_for_move(dentry, target);
> +void mnt_set_violated(struct dentry *root, struct dentry *moving)
> +{
> + struct mountroot *mr;
> + struct mount *mnt;
> +
> + lock_mount_hash();
> + mr = lookup_mountroot(root);
> + if (!mr)
> + goto out;
> +
> + hlist_for_each_entry(mnt, &mr->r_list, mnt_mr_list) {
> + struct mount *child;
> + /* Be wary of this mount */
> + mnt->mnt.mnt_flags |= MNT_VIOLATED;
> +
> + list_for_each_entry(child, &mnt->mnt_mounts, mnt_child) {
> + /* Ignore children that will continue to be connected */
> + if ((child->mnt_mountpoint != moving) &&
> + !d_ancestor(moving, child->mnt_mountpoint))
> + continue;
> +
> + /* Deal with mounts loosing the connection to
> + * their parents
> + */
> + if (!(child->mnt.mnt_flags & MNT_UMOUNT)) {
> + child->mnt.mnt_flags |= MNT_UNREACHABLE_PARENT;
> + hlist_add_head(&child->mnt_pending_umount, &pending_umount);
> + schedule_work(&pending_umount_work);
> + } else {
> + umount_mnt(child);
> + }
> + }
> + }
> +out:
> + unlock_mount_hash();
And that can have non-trivial security implications - ability to expose
something that has been overmounted is potentially very nasty. I might
be missing something in the rest of the series (I'm half-asleep right now,
so that's certainly possible), but that doesn't look obviously safe.
Note that it's very different from the situation with umount-on-invalidation -
there the thing we are uncovering is dead. In this one it might be very
much alive and deliberately covered...
--
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
