Re: [PATCH v4 1/3] nsproxy: add struct nsset
On Tue, May 05, 2020 at 04:04:30PM +0200, Christian Brauner wrote:
> Add a simple struct nsset. It holds all necessary pieces to switch to a new
> set of namespaces without leaving a task in a half-switched state which we
> will make use of in the next patch. This patch switches the existing setns
> logic over without causing a change in setns() behavior. This brings
> setns() closer to how unshare() works(). The prepare_ns() function is
> responsible to prepare all necessary information. This has two reasons.
> First it minimizes dependencies between individual namespaces, i.e. all
> install handler can expect that all fields are properly initialized
> independent in what order they are called in. Second, this makes the code
> easier to maintain and easier to follow if it needs to be changed.
>
> The prepare_ns() helper will only be switched over to use a flags argument
> in the next patch. Here it will still use nstype as a simple integer
> argument which was argued would be clearer. I'm not particularly
> opinionated about this if it really helps or not. The struct nsset itself
> already contains the flags field since its name already indicates that it
> can contain information required by different namespaces. None of this
> should have functional consequences.
>
> Cc: Eric W. Biederman
> Cc: Serge Hallyn
Reviewed-by: Serge Hallyn
Thanks, Christian.
> Cc: Jann Horn
> Cc: Michael Kerrisk
> Cc: Aleksa Sarai
> Signed-off-by: Christian Brauner
> ---
> /* v2 */
> patch introduced
>
> /* v3 */
> - Eric W. Biederman :
> - Remove the prior ns_capable_cred() patch and simplify the permission
> check from ns_capable_cred(nsset, nsset->cred->user_ns, CAP_SYS_ADMIN))
> to from ns_capable(nsset->cred->user_ns, CAP_SYS_ADMIN)).
>
> /* v4 */
> - Eric W. Biederman :
> - Fix nstype == 0 case.
> ---
> fs/namespace.c| 10 ++--
> include/linux/mnt_namespace.h | 1 +
> include/linux/nsproxy.h | 24 ++
> include/linux/proc_ns.h | 4 +-
> ipc/namespace.c | 7 ++-
> kernel/cgroup/namespace.c | 5 +-
> kernel/nsproxy.c | 90 ++-
> kernel/pid_namespace.c| 5 +-
> kernel/time/namespace.c | 5 +-
> kernel/user_namespace.c | 8 ++--
> kernel/utsname.c | 5 +-
> net/core/net_namespace.c | 5 +-
> 12 files changed, 132 insertions(+), 37 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index a28e4db075ed..62899fad4a04 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3954,16 +3954,18 @@ static void mntns_put(struct ns_common *ns)
> put_mnt_ns(to_mnt_ns(ns));
> }
>
> -static int mntns_install(struct nsproxy *nsproxy, struct ns_common *ns)
> +static int mntns_install(struct nsset *nsset, struct ns_common *ns)
> {
> - struct fs_struct *fs = current->fs;
> + struct nsproxy *nsproxy = nsset->nsproxy;
> + struct fs_struct *fs = nsset->fs;
> struct mnt_namespace *mnt_ns = to_mnt_ns(ns), *old_mnt_ns;
> + struct user_namespace *user_ns = nsset->cred->user_ns;
> struct path root;
> int err;
>
> if (!ns_capable(mnt_ns->user_ns, CAP_SYS_ADMIN) ||
> - !ns_capable(current_user_ns(), CAP_SYS_CHROOT) ||
> - !ns_capable(current_user_ns(), CAP_SYS_ADMIN))
> + !ns_capable(user_ns, CAP_SYS_CHROOT) ||
> + !ns_capable(user_ns, CAP_SYS_ADMIN))
> return -EPERM;
>
> if (is_anon_ns(mnt_ns))
> diff --git a/include/linux/mnt_namespace.h b/include/linux/mnt_namespace.h
> index 35942084cd40..007cfa52efb2 100644
> --- a/include/linux/mnt_namespace.h
> +++ b/include/linux/mnt_namespace.h
> @@ -6,6 +6,7 @@
> struct mnt_namespace;
> struct fs_struct;
> struct user_namespace;
> +struct ns_common;
>
> extern struct mnt_namespace *copy_mnt_ns(unsigned long, struct mnt_namespace
> *,
> struct user_namespace *, struct fs_struct *);
> diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
> index 074f395b9ad2..cdb171efc7cb 100644
> --- a/include/linux/nsproxy.h
> +++ b/include/linux/nsproxy.h
> @@ -41,6 +41,30 @@ struct nsproxy {
> };
> extern struct nsproxy init_nsproxy;
>
> +/*
> + * A structure to encompass all bits needed to install
> + * a partial or complete new set of namespaces.
> + *
> + * If a new user namespace is requested cred will
> + * point to a modifiable set of credentials. If a pointer
> + * to a modifiable set is needed nsset_cred() must be
> + * used and tested.
> + */
> +struct nsset {
> + unsigned flags;
> + struct nsproxy *nsproxy;
> + struct fs_struct *fs;
> + const struct cred *cred;
> +};
> +
> +static inline struct cred *nsset_cred(struct nsset *set)
> +{
> + if (set->flags & CLONE_NEWUSER)
> + return (struct cred *)set->cred;
> +
> + return NULL;
> +}
> +
> /*
> * the namespaces access rules are:
> *
> diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h
> index
[PATCH v4 1/3] nsproxy: add struct nsset
Add a simple struct nsset. It holds all necessary pieces to switch to a new
set of namespaces without leaving a task in a half-switched state which we
will make use of in the next patch. This patch switches the existing setns
logic over without causing a change in setns() behavior. This brings
setns() closer to how unshare() works(). The prepare_ns() function is
responsible to prepare all necessary information. This has two reasons.
First it minimizes dependencies between individual namespaces, i.e. all
install handler can expect that all fields are properly initialized
independent in what order they are called in. Second, this makes the code
easier to maintain and easier to follow if it needs to be changed.
The prepare_ns() helper will only be switched over to use a flags argument
in the next patch. Here it will still use nstype as a simple integer
argument which was argued would be clearer. I'm not particularly
opinionated about this if it really helps or not. The struct nsset itself
already contains the flags field since its name already indicates that it
can contain information required by different namespaces. None of this
should have functional consequences.
Cc: Eric W. Biederman
Cc: Serge Hallyn
Cc: Jann Horn
Cc: Michael Kerrisk
Cc: Aleksa Sarai
Signed-off-by: Christian Brauner
---
/* v2 */
patch introduced
/* v3 */
- Eric W. Biederman :
- Remove the prior ns_capable_cred() patch and simplify the permission
check from ns_capable_cred(nsset, nsset->cred->user_ns, CAP_SYS_ADMIN))
to from ns_capable(nsset->cred->user_ns, CAP_SYS_ADMIN)).
/* v4 */
- Eric W. Biederman :
- Fix nstype == 0 case.
---
fs/namespace.c| 10 ++--
include/linux/mnt_namespace.h | 1 +
include/linux/nsproxy.h | 24 ++
include/linux/proc_ns.h | 4 +-
ipc/namespace.c | 7 ++-
kernel/cgroup/namespace.c | 5 +-
kernel/nsproxy.c | 90 ++-
kernel/pid_namespace.c| 5 +-
kernel/time/namespace.c | 5 +-
kernel/user_namespace.c | 8 ++--
kernel/utsname.c | 5 +-
net/core/net_namespace.c | 5 +-
12 files changed, 132 insertions(+), 37 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index a28e4db075ed..62899fad4a04 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3954,16 +3954,18 @@ static void mntns_put(struct ns_common *ns)
put_mnt_ns(to_mnt_ns(ns));
}
-static int mntns_install(struct nsproxy *nsproxy, struct ns_common *ns)
+static int mntns_install(struct nsset *nsset, struct ns_common *ns)
{
- struct fs_struct *fs = current->fs;
+ struct nsproxy *nsproxy = nsset->nsproxy;
+ struct fs_struct *fs = nsset->fs;
struct mnt_namespace *mnt_ns = to_mnt_ns(ns), *old_mnt_ns;
+ struct user_namespace *user_ns = nsset->cred->user_ns;
struct path root;
int err;
if (!ns_capable(mnt_ns->user_ns, CAP_SYS_ADMIN) ||
- !ns_capable(current_user_ns(), CAP_SYS_CHROOT) ||
- !ns_capable(current_user_ns(), CAP_SYS_ADMIN))
+ !ns_capable(user_ns, CAP_SYS_CHROOT) ||
+ !ns_capable(user_ns, CAP_SYS_ADMIN))
return -EPERM;
if (is_anon_ns(mnt_ns))
diff --git a/include/linux/mnt_namespace.h b/include/linux/mnt_namespace.h
index 35942084cd40..007cfa52efb2 100644
--- a/include/linux/mnt_namespace.h
+++ b/include/linux/mnt_namespace.h
@@ -6,6 +6,7 @@
struct mnt_namespace;
struct fs_struct;
struct user_namespace;
+struct ns_common;
extern struct mnt_namespace *copy_mnt_ns(unsigned long, struct mnt_namespace *,
struct user_namespace *, struct fs_struct *);
diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index 074f395b9ad2..cdb171efc7cb 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -41,6 +41,30 @@ struct nsproxy {
};
extern struct nsproxy init_nsproxy;
+/*
+ * A structure to encompass all bits needed to install
+ * a partial or complete new set of namespaces.
+ *
+ * If a new user namespace is requested cred will
+ * point to a modifiable set of credentials. If a pointer
+ * to a modifiable set is needed nsset_cred() must be
+ * used and tested.
+ */
+struct nsset {
+ unsigned flags;
+ struct nsproxy *nsproxy;
+ struct fs_struct *fs;
+ const struct cred *cred;
+};
+
+static inline struct cred *nsset_cred(struct nsset *set)
+{
+ if (set->flags & CLONE_NEWUSER)
+ return (struct cred *)set->cred;
+
+ return NULL;
+}
+
/*
* the namespaces access rules are:
*
diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h
index 6abe85c34681..75807ecef880 100644
--- a/include/linux/proc_ns.h
+++ b/include/linux/proc_ns.h
@@ -8,7 +8,7 @@
#include
struct pid_namespace;
-struct nsproxy;
+struct nsset;
struct path;
struct task_struct;
struct inode;
@@ -19,7 +19,7 @@ struct proc_ns_operations {
int type;
struct ns_common *(*get)(struct
