Skip to content

Commit

Permalink
VFS: Make clone_mnt()/copy_tree()/collect_mounts() return errors
Browse files Browse the repository at this point in the history 
copy_tree() can theoretically fail in a case other than ENOMEM, but always
returns NULL which is interpreted by callers as -ENOMEM.  Change it to return
an explicit error.

Also change clone_mnt() for consistency and because union mounts will add new
error cases.

Thanks to Andreas Gruenbacher <agruen@suse.de> for a bug fix.
[AV: folded braino fix by Dan Carpenter]

Original-author: Valerie Aurora <vaurora@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Cc: Valerie Aurora <valerie.aurora@gmail.com>
Cc: Andreas Gruenbacher <agruen@suse.de>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
  • Loading branch information
David Howells authored and Al Viro committed Jul 14, 2012
1 parent 55e4def commit be34d1a
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 62 deletions.
120 changes: 65 additions & 55 deletions fs/namespace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -708,56 +708,60 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
int flag)
{
struct super_block *sb = old->mnt.mnt_sb;
struct mount *mnt = alloc_vfsmnt(old->mnt_devname);
struct mount *mnt;
int err;

if (mnt) {
if (flag & (CL_SLAVE | CL_PRIVATE))
mnt->mnt_group_id = 0; /* not a peer of original */
else
mnt->mnt_group_id = old->mnt_group_id;

if ((flag & CL_MAKE_SHARED) && !mnt->mnt_group_id) {
int err = mnt_alloc_group_id(mnt);
if (err)
goto out_free;
}
mnt = alloc_vfsmnt(old->mnt_devname);
if (!mnt)
return ERR_PTR(-ENOMEM);

mnt->mnt.mnt_flags = old->mnt.mnt_flags & ~MNT_WRITE_HOLD;
atomic_inc(&sb->s_active);
mnt->mnt.mnt_sb = sb;
mnt->mnt.mnt_root = dget(root);
mnt->mnt_mountpoint = mnt->mnt.mnt_root;
mnt->mnt_parent = mnt;
br_write_lock(&vfsmount_lock);
list_add_tail(&mnt->mnt_instance, &sb->s_mounts);
br_write_unlock(&vfsmount_lock);
if (flag & (CL_SLAVE | CL_PRIVATE))
mnt->mnt_group_id = 0; /* not a peer of original */
else
mnt->mnt_group_id = old->mnt_group_id;

if (flag & CL_SLAVE) {
list_add(&mnt->mnt_slave, &old->mnt_slave_list);
mnt->mnt_master = old;
CLEAR_MNT_SHARED(mnt);
} else if (!(flag & CL_PRIVATE)) {
if ((flag & CL_MAKE_SHARED) || IS_MNT_SHARED(old))
list_add(&mnt->mnt_share, &old->mnt_share);
if (IS_MNT_SLAVE(old))
list_add(&mnt->mnt_slave, &old->mnt_slave);
mnt->mnt_master = old->mnt_master;
}
if (flag & CL_MAKE_SHARED)
set_mnt_shared(mnt);

/* stick the duplicate mount on the same expiry list
* as the original if that was on one */
if (flag & CL_EXPIRE) {
if (!list_empty(&old->mnt_expire))
list_add(&mnt->mnt_expire, &old->mnt_expire);
}
if ((flag & CL_MAKE_SHARED) && !mnt->mnt_group_id) {
err = mnt_alloc_group_id(mnt);
if (err)
goto out_free;
}

mnt->mnt.mnt_flags = old->mnt.mnt_flags & ~MNT_WRITE_HOLD;
atomic_inc(&sb->s_active);
mnt->mnt.mnt_sb = sb;
mnt->mnt.mnt_root = dget(root);
mnt->mnt_mountpoint = mnt->mnt.mnt_root;
mnt->mnt_parent = mnt;
br_write_lock(&vfsmount_lock);
list_add_tail(&mnt->mnt_instance, &sb->s_mounts);
br_write_unlock(&vfsmount_lock);

if (flag & CL_SLAVE) {
list_add(&mnt->mnt_slave, &old->mnt_slave_list);
mnt->mnt_master = old;
CLEAR_MNT_SHARED(mnt);
} else if (!(flag & CL_PRIVATE)) {
if ((flag & CL_MAKE_SHARED) || IS_MNT_SHARED(old))
list_add(&mnt->mnt_share, &old->mnt_share);
if (IS_MNT_SLAVE(old))
list_add(&mnt->mnt_slave, &old->mnt_slave);
mnt->mnt_master = old->mnt_master;
}
if (flag & CL_MAKE_SHARED)
set_mnt_shared(mnt);

/* stick the duplicate mount on the same expiry list
* as the original if that was on one */
if (flag & CL_EXPIRE) {
if (!list_empty(&old->mnt_expire))
list_add(&mnt->mnt_expire, &old->mnt_expire);
}

return mnt;

out_free:
free_vfsmnt(mnt);
return NULL;
return ERR_PTR(err);
}

static inline void mntfree(struct mount *mnt)
Expand Down Expand Up @@ -1242,11 +1246,12 @@ struct mount *copy_tree(struct mount *mnt, struct dentry *dentry,
struct path path;

if (!(flag & CL_COPY_ALL) && IS_MNT_UNBINDABLE(mnt))
return NULL;
return ERR_PTR(-EINVAL);

res = q = clone_mnt(mnt, dentry, flag);
if (!q)
goto Enomem;
if (IS_ERR(q))
return q;

q->mnt_mountpoint = mnt->mnt_mountpoint;

p = mnt;
Expand All @@ -1268,34 +1273,38 @@ struct mount *copy_tree(struct mount *mnt, struct dentry *dentry,
path.mnt = &q->mnt;
path.dentry = p->mnt_mountpoint;
q = clone_mnt(p, p->mnt.mnt_root, flag);
if (!q)
goto Enomem;
if (IS_ERR(q))
goto out;
br_write_lock(&vfsmount_lock);
list_add_tail(&q->mnt_list, &res->mnt_list);
attach_mnt(q, &path);
br_write_unlock(&vfsmount_lock);
}
}
return res;
Enomem:
out:
if (res) {
LIST_HEAD(umount_list);
br_write_lock(&vfsmount_lock);
umount_tree(res, 0, &umount_list);
br_write_unlock(&vfsmount_lock);
release_mounts(&umount_list);
}
return NULL;
return q;
}

/* Caller should check returned pointer for errors */

struct vfsmount *collect_mounts(struct path *path)
{
struct mount *tree;
down_write(&namespace_sem);
tree = copy_tree(real_mount(path->mnt), path->dentry,
CL_COPY_ALL | CL_PRIVATE);
up_write(&namespace_sem);
return tree ? &tree->mnt : NULL;
if (IS_ERR(tree))
return NULL;
return &tree->mnt;
}

void drop_collected_mounts(struct vfsmount *mnt)
Expand Down Expand Up @@ -1590,14 +1599,15 @@ static int do_loopback(struct path *path, char *old_name,
if (!check_mnt(real_mount(path->mnt)) || !check_mnt(old))
goto out2;

err = -ENOMEM;
if (recurse)
mnt = copy_tree(old, old_path.dentry, 0);
else
mnt = clone_mnt(old, old_path.dentry, 0);

if (!mnt)
goto out2;
if (IS_ERR(mnt)) {
err = PTR_ERR(mnt);
goto out;
}

err = graft_tree(mnt, path);
if (err) {
Expand Down Expand Up @@ -2211,10 +2221,10 @@ static struct mnt_namespace *dup_mnt_ns(struct mnt_namespace *mnt_ns,
down_write(&namespace_sem);
/* First pass: copy the tree topology */
new = copy_tree(old, old->mnt.mnt_root, CL_COPY_ALL | CL_EXPIRE);
if (!new) {
if (IS_ERR(new)) {
up_write(&namespace_sem);
kfree(new_ns);
return ERR_PTR(-ENOMEM);
return ERR_CAST(new);
}
new_ns->root = new;
br_write_lock(&vfsmount_lock);
Expand Down
5 changes: 3 additions & 2 deletions fs/pnode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,9 @@ int propagate_mnt(struct mount *dest_mnt, struct dentry *dest_dentry,

source = get_source(m, prev_dest_mnt, prev_src_mnt, &type);

if (!(child = copy_tree(source, source->mnt.mnt_root, type))) {
ret = -ENOMEM;
child = copy_tree(source, source->mnt.mnt_root, type);
if (IS_ERR(child)) {
ret = PTR_ERR(child);
list_splice(tree_list, tmp_list.prev);
goto out;
}
Expand Down
10 changes: 5 additions & 5 deletions kernel/audit_tree.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ void audit_trim_trees(void)

root_mnt = collect_mounts(&path);
path_put(&path);
if (!root_mnt)
if (IS_ERR(root_mnt))
goto skip_it;

spin_lock(&hash_lock);
Expand Down Expand Up @@ -669,8 +669,8 @@ int audit_add_tree_rule(struct audit_krule *rule)
goto Err;
mnt = collect_mounts(&path);
path_put(&path);
if (!mnt) {
err = -ENOMEM;
if (IS_ERR(mnt)) {
err = PTR_ERR(mnt);
goto Err;
}

Expand Down Expand Up @@ -719,8 +719,8 @@ int audit_tag_tree(char *old, char *new)
return err;
tagged = collect_mounts(&path2);
path_put(&path2);
if (!tagged)
return -ENOMEM;
if (IS_ERR(tagged))
return PTR_ERR(tagged);

err = kern_path(old, 0, &path1);
if (err) {
Expand Down

0 comments on commit be34d1a

Please sign in to comment.