Serge E. Hallyn <[EMAIL PROTECTED]> wrote:
> + new->uid = new->euid = new->suid = new->fsuid = 0;
> + new->gid = new->egid = new->sgid = new->fsgid = 0;
Should the supplementary groups be zapped too? Do the GIDs therein still have
meaning in the new user namespace?
Note also that eCryptFS is broken by your patch.
I suggest adding the attached incremental patch. It makes the following
changes:
(1) Provides a current_user_ns() macro to wrap accesses to current's user
namespace.
(2) Fixes eCryptFS.
(3) Renames create_new_userns() to create_user_ns() to be more consistent
with the other associated functions and because the 'new' in the name is
superfluous.
(4) Moves the argument and permission checks made for CLONE_NEWUSER to the
beginning of do_fork() so that they're done prior to making any attempts
at allocation.
(5) Calls create_user_ns() after prepare_creds(), and gives it the new creds
to fill in rather than have it return the new root user. I don't imagine
the new root user being used for anything other than filling in a cred
struct.
This also permits me to get rid of a get_uid() and a free_uid(), as the
reference the creds were holding on the old user_struct can just be
transferred to the new namespace's creator pointer.
(6) Makes create_user_ns() reset the UIDs and GIDs of the creds under
preparation rather than doing it in copy_creds().
David
---
diff --git a/fs/ecryptfs/messaging.c b/fs/ecryptfs/messaging.c
index 92bf606..eecb8b5 100644
--- a/fs/ecryptfs/messaging.c
+++ b/fs/ecryptfs/messaging.c
@@ -376,7 +376,7 @@ int ecryptfs_process_response(struct ecryptfs_message *msg,
uid_t euid,
struct ecryptfs_msg_ctx *msg_ctx;
size_t msg_size;
struct nsproxy *nsproxy;
- struct user_namespace *current_user_ns;
+ struct user_namespace *tsk_user_ns;
uid_t ctx_euid;
int rc;
@@ -401,9 +401,9 @@ int ecryptfs_process_response(struct ecryptfs_message *msg,
uid_t euid,
mutex_unlock(&ecryptfs_daemon_hash_mux);
goto wake_up;
}
- current_user_ns = nsproxy->user_ns;
+ tsk_user_ns = __task_cred(msg_ctx->task)->user->user_ns;
ctx_euid = task_euid(msg_ctx->task);
- rc = ecryptfs_find_daemon_by_euid(&daemon, ctx_euid, current_user_ns);
+ rc = ecryptfs_find_daemon_by_euid(&daemon, ctx_euid, tsk_user_ns);
rcu_read_unlock();
mutex_unlock(&ecryptfs_daemon_hash_mux);
if (rc) {
@@ -421,11 +421,11 @@ int ecryptfs_process_response(struct ecryptfs_message
*msg, uid_t euid,
euid, ctx_euid);
goto unlock;
}
- if (current_user_ns != user_ns) {
+ if (tsk_user_ns != user_ns) {
rc = -EBADMSG;
printk(KERN_WARNING "%s: Received message from user_ns "
"[0x%p]; expected message from user_ns [0x%p]\n",
- __func__, user_ns, nsproxy->user_ns);
+ __func__, user_ns, tsk_user_ns);
goto unlock;
}
if (daemon->pid != pid) {
@@ -486,8 +486,7 @@ ecryptfs_send_message_locked(unsigned int transport, char
*data, int data_len,
uid_t euid = current_euid();
int rc;
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid,
- current->nsproxy->user_ns);
+ rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
if (rc || !daemon) {
rc = -ENOTCONN;
printk(KERN_ERR "%s: User [%d] does not have a daemon "
diff --git a/fs/ecryptfs/miscdev.c b/fs/ecryptfs/miscdev.c
index 047ac60..efd95a0 100644
--- a/fs/ecryptfs/miscdev.c
+++ b/fs/ecryptfs/miscdev.c
@@ -47,8 +47,7 @@ ecryptfs_miscdev_poll(struct file *file, poll_table *pt)
mutex_lock(&ecryptfs_daemon_hash_mux);
/* TODO: Just use file->private_data? */
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid,
- current->nsproxy->user_ns);
+ rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
BUG_ON(rc || !daemon);
mutex_lock(&daemon->mux);
mutex_unlock(&ecryptfs_daemon_hash_mux);
@@ -95,11 +94,9 @@ ecryptfs_miscdev_open(struct inode *inode, struct file *file)
"count; rc = [%d]\n", __func__, rc);
goto out_unlock_daemon_list;
}
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid,
- current->nsproxy->user_ns);
+ rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
if (rc || !daemon) {
- rc = ecryptfs_spawn_daemon(&daemon, euid,
- current->nsproxy->user_ns,
+ rc = ecryptfs_spawn_daemon(&daemon, euid, current_user_ns(),
task_pid(current));
if (rc) {
printk(KERN_ERR "%s: Error attempting to spawn daemon; "
@@ -153,8 +150,7 @@ ecryptfs_miscdev_release(struct inode *inode, struct file
*file)
int rc;
mutex_lock(&ecryptfs_daemon_hash_mux);
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid,
- current->nsproxy->user_ns);
+ rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
BUG_ON(rc || !daemon);
mutex_lock(&daemon->mux);
BUG_ON(daemon->pid != task_pid(current));
@@ -254,8 +250,7 @@ ecryptfs_miscdev_read(struct file *file, char __user *buf,
size_t count,
mutex_lock(&ecryptfs_daemon_hash_mux);
/* TODO: Just use file->private_data? */
- rc = ecryptfs_find_daemon_by_euid(&daemon, euid,
- current->nsproxy->user_ns);
+ rc = ecryptfs_find_daemon_by_euid(&daemon, euid, current_user_ns());
BUG_ON(rc || !daemon);
mutex_lock(&daemon->mux);
if (daemon->flags & ECRYPTFS_DAEMON_ZOMBIE) {
@@ -295,7 +290,7 @@ check_list:
goto check_list;
}
BUG_ON(euid != daemon->euid);
- BUG_ON(current->nsproxy->user_ns != daemon->user_ns);
+ BUG_ON(current_user_ns() != daemon->user_ns);
BUG_ON(task_pid(current) != daemon->pid);
msg_ctx = list_first_entry(&daemon->msg_ctx_out_queue,
struct ecryptfs_msg_ctx, daemon_out_list);
@@ -468,7 +463,7 @@ ecryptfs_miscdev_write(struct file *file, const char __user
*buf,
goto out_free;
}
rc = ecryptfs_miscdev_response(&data[i], packet_size,
- euid, current->nsproxy->user_ns,
+ euid, current_user_ns(),
task_pid(current), seq);
if (rc)
printk(KERN_WARNING "%s: Failed to deliver miscdev "
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 26c1ab1..7db0049 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -315,6 +315,7 @@ static inline void put_cred(const struct cred *_cred)
#define current_fsgid() (current_cred_xxx(fsgid))
#define current_cap() (current_cred_xxx(cap_effective))
#define current_user() (current_cred_xxx(user))
+#define current_user_ns() (current_cred_xxx(user)->user_ns)
#define current_security() (current_cred_xxx(security))
#define current_uid_gid(_uid, _gid) \
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index d6e61a2..315bcd3 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -26,7 +26,7 @@ static inline struct user_namespace *get_user_ns(struct
user_namespace *ns)
return ns;
}
-extern struct user_struct *create_new_userns(struct task_struct *tsk);
+extern int create_user_ns(struct cred *new);
extern void free_user_ns(struct kref *kref);
static inline void put_user_ns(struct user_namespace *ns)
@@ -42,9 +42,9 @@ static inline struct user_namespace *get_user_ns(struct
user_namespace *ns)
return &init_user_ns;
}
-static inline struct user_struct *create_new_userns(struct task_struct *tsk)
+static inline int create_user_ns(struct cred *new)
{
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
static inline void put_user_ns(struct user_namespace *ns)
diff --git a/kernel/cred.c b/kernel/cred.c
index e98106e..d0f99d8 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -274,7 +274,7 @@ int copy_creds(struct task_struct *p, unsigned long
clone_flags)
struct thread_group_cred *tgcred;
#endif
struct cred *new;
- struct user_struct *new_root = NULL;
+ int ret;
mutex_init(&p->cred_exec_mutex);
@@ -289,33 +289,14 @@ int copy_creds(struct task_struct *p, unsigned long
clone_flags)
return 0;
}
- if (clone_flags & CLONE_NEWUSER) {
- /*
- * hopefully the capability check goes away when userns support
- * is complete
- */
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- if (clone_flags & CLONE_THREAD)
- return -EINVAL;
- new_root = create_new_userns(p);
- if (IS_ERR(new_root))
- return PTR_ERR(new_root);
- }
-
new = prepare_creds();
- if (!new) {
- free_uid(new_root);
+ if (!new)
return -ENOMEM;
- }
- /* If we created a new user_ns, make its root user
- * our user */
- if (new_root) {
- new->uid = new->euid = new->suid = new->fsuid = 0;
- new->gid = new->egid = new->sgid = new->fsgid = 0;
- free_uid(new->user);
- new->user = new_root;
+ if (clone_flags & CLONE_NEWUSER) {
+ ret = create_user_ns(new);
+ if (ret < 0)
+ goto error_put;
}
#ifdef CONFIG_KEYS
@@ -333,10 +314,8 @@ int copy_creds(struct task_struct *p, unsigned long
clone_flags)
* bit */
if (!(clone_flags & CLONE_THREAD)) {
tgcred = kmalloc(sizeof(*tgcred), GFP_KERNEL);
- if (!tgcred) {
- put_cred(new);
- return -ENOMEM;
- }
+ if (!tgcred)
+ goto nomem_put;
atomic_set(&tgcred->usage, 1);
spin_lock_init(&tgcred->lock);
tgcred->process_keyring = NULL;
@@ -350,6 +329,12 @@ int copy_creds(struct task_struct *p, unsigned long
clone_flags)
atomic_inc(&new->user->processes);
p->cred = p->real_cred = get_cred(new);
return 0;
+
+nomem_put:
+ ret = -ENOMEM;
+error_put:
+ put_cred(new);
+ return ret;
}
/**
diff --git a/kernel/fork.c b/kernel/fork.c
index c3bb673..2e167d5 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1318,6 +1318,20 @@ long do_fork(unsigned long clone_flags,
long nr;
/*
+ * Do some preliminary argument and permissions checking before we
+ * actually start allocating stuff
+ */
+ if (clone_flags & CLONE_NEWUSER) {
+ if (clone_flags & CLONE_THREAD)
+ return -EINVAL;
+ /* hopefully this check will go away when userns support is
+ * complete
+ */
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ }
+
+ /*
* We hope to recycle these flags after 2.6.26
*/
if (unlikely(clone_flags & CLONE_STOPPED)) {
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 86200b1..5da3c41 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -12,10 +12,14 @@
#include <linux/cred.h>
/*
- * Return a new user_struct which is root in a new user_ns. This is called by
- * copy_creds(), which will finish setting the target task's credentials.
+ * Create a new user namespace, deriving the creator from the user in the
+ * passed credentials, and replacing that user with the new root user for the
+ * new namespace.
+ *
+ * This is called by copy_creds(), which will finish setting the target task's
+ * credentials.
*/
-struct user_struct *create_new_userns(struct task_struct *tsk)
+int create_user_ns(struct cred *new)
{
struct user_namespace *ns;
struct user_struct *root_user;
@@ -23,7 +27,7 @@ struct user_struct *create_new_userns(struct task_struct *tsk)
ns = kmalloc(sizeof(struct user_namespace), GFP_KERNEL);
if (!ns)
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
kref_init(&ns->kref);
@@ -34,18 +38,20 @@ struct user_struct *create_new_userns(struct task_struct
*tsk)
root_user = alloc_uid(ns, 0);
if (!root_user) {
kfree(ns);
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;
}
- /* save away and pin the creating user */
- ns->creator = tsk->cred->user; /* tsk is still being created */
- get_uid(ns->creator);
+ /* set the new root user in the credentials under preparation */
+ ns->creator = new->user;
+ new->user = root_user;
+ new->uid = new->euid = new->suid = new->fsuid = 0;
+ new->gid = new->egid = new->sgid = new->fsgid = 0;
/* alloc_uid() incremented the userns refcount. Just set it to 1 */
kref_set(&ns->kref, 1);
printk(KERN_NOTICE "allocated a user_ns (%p)\n", ns);
- return root_user;
+ return 0;
}
void free_user_ns(struct kref *kref)
_______________________________________________
Containers mailing list
[EMAIL PROTECTED]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
