Re: [patch 4/8] propagate error values from clone_mnt

[Eric W. Biederman]

Miklos Szeredi <[EMAIL PROTECTED]> writes:

> From: Miklos Szeredi <[EMAIL PROTECTED]>
>
> Allow clone_mnt() to return errors other than ENOMEM.  This will be
> used for returning a different error value when the number of user
> mounts goes over the limit.
>
> Fix copy_tree() to return EPERM for unbindable mounts.
>
> Don't propagate further from dup_mnt_ns() as that copy_tree() can only
> fail with -ENOMEM.

At a quick skim this looks ok.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[patch 4/8] propagate error values from clone_mnt

[Miklos Szeredi]

From: Miklos Szeredi <[EMAIL PROTECTED]>

Allow clone_mnt() to return errors other than ENOMEM.  This will be
used for returning a different error value when the number of user
mounts goes over the limit.

Fix copy_tree() to return EPERM for unbindable mounts.

Don't propagate further from dup_mnt_ns() as that copy_tree() can only
fail with -ENOMEM.

Signed-off-by: Miklos Szeredi <[EMAIL PROTECTED]>
---

Index: linux/fs/namespace.c
===
--- linux.orig/fs/namespace.c   2007-04-20 11:55:07.0 +0200
+++ linux/fs/namespace.c2007-04-20 11:55:09.0 +0200
@@ -261,42 +261,42 @@ static struct vfsmount *clone_mnt(struct
 {
struct super_block *sb = old->mnt_sb;
struct vfsmount *mnt = alloc_vfsmnt(old->mnt_devname);
+   if (!mnt)
+   return ERR_PTR(-ENOMEM);
 
-   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;
-
-   /* don't copy the MNT_USER flag */
-   mnt->mnt_flags &= ~MNT_USER;
-   if (flag & CL_SETUSER)
-   set_mnt_user(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);
+   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;
+
+   /* don't copy the MNT_USER flag */
+   mnt->mnt_flags &= ~MNT_USER;
+   if (flag & CL_SETUSER)
+   set_mnt_user(mnt);
 
-   /* 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 (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);
+
+   /* 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;
 }
@@ -781,11 +781,11 @@ struct vfsmount *copy_tree(struct vfsmou
struct nameidata nd;
 
if (!(flag & CL_COPY_ALL) && IS_MNT_UNBINDABLE(mnt))
-   return NULL;
+   return ERR_PTR(-EPERM);
 
res = q = clone_mnt(mnt, dentry, flag);
-   if (!q)
-   goto Enomem;
+   if (IS_ERR(q))
+   goto error;
q->mnt_mountpoint = mnt->mnt_mountpoint;
 
p = mnt;
@@ -806,8 +806,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)
-   goto Enomem;
+   if (IS_ERR(q))
+   goto error;
spin_lock(&vfsmount_lock);
list_add_tail(&q->mnt_list, &res->mnt_list);
attach_mnt(q, &nd);
@@ -815,7 +815,7 @@ struct vfsmount *copy_tree(struct vfsmou
}
}
return res;
-Enomem:
+ error:
if (res) {
LIST_HEAD(umount_list);
spin_lock(&vfsmount_lock);
@@ -823,7 +823,7 @@ Enomem:
s