| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20070104193555.GA17506@sergelap.austin.ibm.com>
Date: Thu, 4 Jan 2007 13:35:55 -0600
From: "Serge E. Hallyn" <serue@...ibm.com>
To: Frederik Deweerdt <deweerdt@...e.fr>
Cc: "Serge E. Hallyn" <serue@...ibm.com>,
Andrew Morton <akpm@...l.org>,
lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -mm 5/8] user ns: prepare copy_tree, copy_mnt, and their callers to handle errs
Quoting Frederik Deweerdt (deweerdt@...e.fr):
> On Thu, Jan 04, 2007 at 12:12:15PM -0600, Serge E. Hallyn wrote:
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index 5da87e2..a4039a3 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -708,8 +708,9 @@ struct vfsmount *copy_tree(struct vfsmou
> > return NULL;
> >
> > res = q = clone_mnt(mnt, dentry, flag);
> > - if (!q)
> > - goto Enomem;
> > + if (!q || IS_ERR(q)) {
> > + return q;
> > + }
> > q->mnt_mountpoint = mnt->mnt_mountpoint;
> >
> > p = mnt;
> > @@ -730,8 +731,9 @@ struct vfsmount *copy_tree(struct vfsmou
> > nd.mnt = q;
> > nd.dentry = p->mnt_mountpoint;
> > q = clone_mnt(p, p->mnt_root, flag);
> > - if (!q)
> > - goto Enomem;
> > + if (!q || IS_ERR(q)) {
> > + goto Error;
> > + }
> > spin_lock(&vfsmount_lock);
> > list_add_tail(&q->mnt_list, &res->mnt_list);
> > attach_mnt(q, &nd);
> > @@ -739,7 +741,7 @@ struct vfsmount *copy_tree(struct vfsmou
> > }
> > }
> > return res;
> > -Enomem:
> > +Error:
> > if (res) {
> ^^^^^^^^^^
> I think that this check can be safely skiped, as res is always non-NULL
> when Error: is now reached, isn't it?
Good point. In addition, we can clean the code up a little by not
letting clone_mnt return NULL. Compile-tested patch follows. Though
really it starts to look like clone_mnt should be split into two
parts, with the original clone_mnt static inline, and the new clone_mnt
doing the permission check and return value conversion...
From: Serge E. Hallyn <serue@...ibm.com>
Subject: [PATCH 1/1] userns: cleanups with clone_mnt
As Frederik Deweerdt points out, the Error: case in copy_tree()
can no longer have res==NULL, so remove that check.
Also make clone_mnt always return -ENOMEM in place of NULL.
Signed-off-by: Serge E. Hallyn <serue@...ibm.com>
---
fs/namespace.c | 90 ++++++++++++++++++++++++++++----------------------------
1 files changed, 45 insertions(+), 45 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 60ca9b5..2ed89d4 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -243,41 +243,43 @@ static struct vfsmount *clone_mnt(struct
mnt = alloc_vfsmnt(old->mnt_devname);
- if (mnt) {
- 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_PROPAGATION) || 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);
- if (flag & CL_SHARE_NS)
- mnt->mnt_flags |= MNT_SHARE_NS;
- else
- mnt->mnt_flags &= ~MNT_SHARE_NS;
-
- /* stick the duplicate mount on the same expiry list
- * as the original if that was on one */
- if (flag & CL_EXPIRE) {
- spin_lock(&vfsmount_lock);
- if (!list_empty(&old->mnt_expire))
- list_add(&mnt->mnt_expire, &old->mnt_expire);
- spin_unlock(&vfsmount_lock);
- }
+ 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_PROPAGATION) || 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);
+ if (flag & CL_SHARE_NS)
+ mnt->mnt_flags |= MNT_SHARE_NS;
+ else
+ mnt->mnt_flags &= ~MNT_SHARE_NS;
+
+ /* stick the duplicate mount on the same expiry list
+ * as the original if that was on one */
+ if (flag & CL_EXPIRE) {
+ spin_lock(&vfsmount_lock);
+ if (!list_empty(&old->mnt_expire))
+ list_add(&mnt->mnt_expire, &old->mnt_expire);
+ spin_unlock(&vfsmount_lock);
+ }
+
return mnt;
}
@@ -715,14 +717,15 @@ struct vfsmount *copy_tree(struct vfsmou
{
struct vfsmount *res, *p, *q, *r, *s;
struct nameidata nd;
+ LIST_HEAD(umount_list);
if (!(flag & CL_COPY_ALL) && IS_MNT_UNBINDABLE(mnt))
return NULL;
res = q = clone_mnt(mnt, dentry, flag);
- if (!q || IS_ERR(q)) {
+ if (IS_ERR(q))
return q;
- }
+
q->mnt_mountpoint = mnt->mnt_mountpoint;
p = mnt;
@@ -743,9 +746,8 @@ struct vfsmount *copy_tree(struct vfsmou
nd.mnt = q;
nd.dentry = p->mnt_mountpoint;
q = clone_mnt(p, p->mnt_root, flag);
- if (!q || IS_ERR(q)) {
+ if (IS_ERR(q))
goto Error;
- }
spin_lock(&vfsmount_lock);
list_add_tail(&q->mnt_list, &res->mnt_list);
attach_mnt(q, &nd);
@@ -754,13 +756,11 @@ struct vfsmount *copy_tree(struct vfsmou
}
return res;
Error:
- if (res) {
- LIST_HEAD(umount_list);
- spin_lock(&vfsmount_lock);
- umount_tree(res, 0, &umount_list);
- spin_unlock(&vfsmount_lock);
- release_mounts(&umount_list);
- }
+ spin_lock(&vfsmount_lock);
+ umount_tree(res, 0, &umount_list);
+ spin_unlock(&vfsmount_lock);
+ release_mounts(&umount_list);
+
return q;
}
--
1.4.1
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists