From: Valerie Aurora <vaurora@xxxxxxxxxx> copy_tree() can theoretically fail in a case other than ENOMEM, but always returns NULL which is interpreted by callers as -ENOMEM. Convert to return an explicit error. Convert clone_mnt() for consistency and because union mounts will add new error cases. Thanks to Andreas Gruenbacher <agruen@xxxxxxx> for a bug fix. Cc: Andreas Gruenbacher <agruen@xxxxxxx> Signed-off-by: Valerie Aurora <valerie.aurora@xxxxxxxxx> --- fs/namespace.c | 111 +++++++++++++++++++++++++++----------------------- fs/pnode.c | 5 +- kernel/audit_tree.c | 10 ++-- 3 files changed, 68 insertions(+), 58 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 745feaf..79be922 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -593,53 +593,57 @@ static struct vfsmount *clone_mnt(struct vfsmount *old, struct dentry *root, int flag) { struct super_block *sb = old->mnt_sb; - struct vfsmount *mnt = alloc_vfsmnt(old->mnt_devname); + struct vfsmount *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_flags = old->mnt_flags; - atomic_inc(&sb->s_active); - mnt->mnt_sb = sb; - mnt->mnt_root = dget(root); - mnt->mnt_mountpoint = mnt->mnt_root; - mnt->mnt_parent = mnt; - - 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_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) { + err = mnt_alloc_group_id(mnt); + if (err) + goto out_free; } + + mnt->mnt_flags = old->mnt_flags; + atomic_inc(&sb->s_active); + mnt->mnt_sb = sb; + mnt->mnt_root = dget(root); + mnt->mnt_mountpoint = mnt->mnt_root; + mnt->mnt_parent = mnt; + + 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 __mntput(struct vfsmount *mnt) @@ -1255,11 +1259,12 @@ struct vfsmount *copy_tree(struct vfsmount *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; @@ -1280,8 +1285,8 @@ struct vfsmount *copy_tree(struct vfsmount *mnt, struct dentry *dentry, path.mnt = q; path.dentry = p->mnt_mountpoint; q = clone_mnt(p, p->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); @@ -1289,7 +1294,7 @@ struct vfsmount *copy_tree(struct vfsmount *mnt, struct dentry *dentry, } } return res; -Enomem: +out: if (res) { LIST_HEAD(umount_list); br_write_lock(vfsmount_lock); @@ -1297,9 +1302,11 @@ Enomem: 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 vfsmount *tree; @@ -1574,14 +1581,15 @@ static int do_loopback(struct path *path, char *old_name, if (!check_mnt(path->mnt) || !check_mnt(old_path.mnt)) goto out; - err = -ENOMEM; if (recurse) mnt = copy_tree(old_path.mnt, old_path.dentry, 0); else mnt = clone_mnt(old_path.mnt, old_path.dentry, 0); - if (!mnt) + if (IS_ERR(mnt)) { + err = PTR_ERR(mnt); goto out; + } err = graft_tree(mnt, path); if (err) { @@ -2119,10 +2127,11 @@ static struct mnt_namespace *dup_mnt_ns(struct mnt_namespace *mnt_ns, /* First pass: copy the tree topology */ new_ns->root = copy_tree(mnt_ns->root, mnt_ns->root->mnt_root, CL_COPY_ALL | CL_EXPIRE); - if (!new_ns->root) { + if (IS_ERR(new_ns->root)) { + int err = PTR_ERR(new_ns->root); up_write(&namespace_sem); kfree(new_ns); - return ERR_PTR(-ENOMEM); + return ERR_PTR(err); } br_write_lock(vfsmount_lock); list_add_tail(&new_ns->list, &new_ns->root->mnt_list); diff --git a/fs/pnode.c b/fs/pnode.c index 8066b8d..0710bb9 100644 --- a/fs/pnode.c +++ b/fs/pnode.c @@ -253,8 +253,9 @@ int propagate_mnt(struct vfsmount *dest_mnt, struct dentry *dest_dentry, source = get_source(m, prev_dest_mnt, prev_src_mnt, &type); - if (!(child = copy_tree(source, source->mnt_root, type))) { - ret = -ENOMEM; + child = copy_tree(source, source->mnt_root, type); + if (IS_ERR(child)) { + ret = PTR_ERR(child); list_splice(tree_list, tmp_list.prev); goto out; } diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index 7f18d3a..d32ca7a 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -596,7 +596,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); @@ -670,8 +670,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; } @@ -720,8 +720,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) { -- 1.7.0.4 -- 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