Skip to content

Commit

Permalink
User namespaces: set of cleanups (v2)
Browse files Browse the repository at this point in the history
The user_ns is moved from nsproxy to user_struct, so that a struct
cred by itself is sufficient to determine access (which it otherwise
would not be).  Corresponding ecryptfs fixes (by David Howells) are
here as well.

Fix refcounting.  The following rules now apply:
        1. The task pins the user struct.
        2. The user struct pins its user namespace.
        3. The user namespace pins the struct user which created it.

User namespaces are cloned during copy_creds().  Unsharing a new user_ns
is no longer possible.  (We could re-add that, but it'll cause code
duplication and doesn't seem useful if PAM doesn't need to clone user
namespaces).

When a user namespace is created, its first user (uid 0) gets empty
keyrings and a clean group_info.

This incorporates a previous patch by David Howells.  Here
is his original patch description:

>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

>Signed-off-by: David Howells <dhowells@redhat.com>

Changelog:
	Oct 20: integrate dhowells comments
		1. leave thread_keyring alone
		2. use current_user_ns() in set_user()

Signed-off-by: Serge Hallyn <serue@us.ibm.com>
  • Loading branch information
Serge Hallyn committed Nov 24, 2008
1 parent 9789cfe commit 18b6e04
Show file tree
Hide file tree
Showing 13 changed files with 96 additions and 129 deletions.
13 changes: 6 additions & 7 deletions fs/ecryptfs/messaging.c
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,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;

Expand All @@ -385,9 +385,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) {
Expand All @@ -405,11 +405,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) {
Expand Down Expand Up @@ -468,8 +468,7 @@ ecryptfs_send_message_locked(char *data, int data_len, u8 msg_type,
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 "
Expand Down
19 changes: 7 additions & 12 deletions fs/ecryptfs/miscdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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; "
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -295,7 +290,7 @@ ecryptfs_miscdev_read(struct file *file, char __user *buf, size_t count,
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);
Expand Down Expand Up @@ -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 "
Expand Down
2 changes: 2 additions & 0 deletions include/linux/cred.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ do { \
} while (0)

extern struct group_info *groups_alloc(int);
extern struct group_info init_groups;
extern void groups_free(struct group_info *);
extern int set_current_groups(struct group_info *);
extern int set_groups(struct cred *, struct group_info *);
Expand Down Expand Up @@ -315,6 +316,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) \
Expand Down
1 change: 0 additions & 1 deletion include/linux/init_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ extern struct nsproxy init_nsproxy;
.mnt_ns = NULL, \
INIT_NET_NS(net_ns) \
INIT_IPC_NS(ipc_ns) \
.user_ns = &init_user_ns, \
}

#define INIT_SIGHAND(sighand) { \
Expand Down
1 change: 0 additions & 1 deletion include/linux/nsproxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ struct nsproxy {
struct ipc_namespace *ipc_ns;
struct mnt_namespace *mnt_ns;
struct pid_namespace *pid_ns;
struct user_namespace *user_ns;
struct net *net_ns;
};
extern struct nsproxy init_nsproxy;
Expand Down
1 change: 1 addition & 0 deletions include/linux/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ struct user_struct {
/* Hash table maintenance information */
struct hlist_node uidhash_node;
uid_t uid;
struct user_namespace *user_ns;

#ifdef CONFIG_USER_SCHED
struct task_group *tg;
Expand Down
13 changes: 4 additions & 9 deletions include/linux/user_namespace.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
struct user_namespace {
struct kref kref;
struct hlist_head uidhash_table[UIDHASH_SZ];
struct user_struct *root_user;
struct user_struct *creator;
};

extern struct user_namespace init_user_ns;
Expand All @@ -26,8 +26,7 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
return ns;
}

extern struct user_namespace *copy_user_ns(int flags,
struct user_namespace *old_ns);
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)
Expand All @@ -43,13 +42,9 @@ static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
return &init_user_ns;
}

static inline struct user_namespace *copy_user_ns(int flags,
struct user_namespace *old_ns)
static inline int create_user_ns(struct cred *new)
{
if (flags & CLONE_NEWUSER)
return ERR_PTR(-EINVAL);

return old_ns;
return -EINVAL;
}

static inline void put_user_ns(struct user_namespace *ns)
Expand Down
15 changes: 13 additions & 2 deletions kernel/cred.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ int copy_creds(struct task_struct *p, unsigned long clone_flags)
struct thread_group_cred *tgcred;
#endif
struct cred *new;
int ret;

mutex_init(&p->cred_exec_mutex);

Expand All @@ -293,6 +294,12 @@ int copy_creds(struct task_struct *p, unsigned long clone_flags)
if (!new)
return -ENOMEM;

if (clone_flags & CLONE_NEWUSER) {
ret = create_user_ns(new);
if (ret < 0)
goto error_put;
}

#ifdef CONFIG_KEYS
/* new threads get their own thread keyrings if their parent already
* had one */
Expand All @@ -309,8 +316,8 @@ int copy_creds(struct task_struct *p, unsigned long clone_flags)
if (!(clone_flags & CLONE_THREAD)) {
tgcred = kmalloc(sizeof(*tgcred), GFP_KERNEL);
if (!tgcred) {
put_cred(new);
return -ENOMEM;
ret = -ENOMEM;
goto error_put;
}
atomic_set(&tgcred->usage, 1);
spin_lock_init(&tgcred->lock);
Expand All @@ -325,6 +332,10 @@ 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;

error_put:
put_cred(new);
return ret;
}

/**
Expand Down
19 changes: 16 additions & 3 deletions kernel/fork.c
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
if (atomic_read(&p->real_cred->user->processes) >=
p->signal->rlim[RLIMIT_NPROC].rlim_cur) {
if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_RESOURCE) &&
p->real_cred->user != current->nsproxy->user_ns->root_user)
p->real_cred->user != INIT_USER)
goto bad_fork_free;
}

Expand Down Expand Up @@ -1334,6 +1334,20 @@ long do_fork(unsigned long clone_flags,
int trace = 0;
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
*/
Expand Down Expand Up @@ -1581,8 +1595,7 @@ asmlinkage long sys_unshare(unsigned long unshare_flags)
err = -EINVAL;
if (unshare_flags & ~(CLONE_THREAD|CLONE_FS|CLONE_NEWNS|CLONE_SIGHAND|
CLONE_VM|CLONE_FILES|CLONE_SYSVSEM|
CLONE_NEWUTS|CLONE_NEWIPC|CLONE_NEWUSER|
CLONE_NEWNET))
CLONE_NEWUTS|CLONE_NEWIPC|CLONE_NEWNET))
goto bad_unshare_out;

/*
Expand Down
15 changes: 2 additions & 13 deletions kernel/nsproxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ static struct nsproxy *create_new_namespaces(unsigned long flags,
goto out_pid;
}

new_nsp->user_ns = copy_user_ns(flags, tsk->nsproxy->user_ns);
if (IS_ERR(new_nsp->user_ns)) {
err = PTR_ERR(new_nsp->user_ns);
goto out_user;
}

new_nsp->net_ns = copy_net_ns(flags, tsk->nsproxy->net_ns);
if (IS_ERR(new_nsp->net_ns)) {
err = PTR_ERR(new_nsp->net_ns);
Expand All @@ -95,9 +89,6 @@ static struct nsproxy *create_new_namespaces(unsigned long flags,
return new_nsp;

out_net:
if (new_nsp->user_ns)
put_user_ns(new_nsp->user_ns);
out_user:
if (new_nsp->pid_ns)
put_pid_ns(new_nsp->pid_ns);
out_pid:
Expand Down Expand Up @@ -130,7 +121,7 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk)
get_nsproxy(old_ns);

if (!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
CLONE_NEWUSER | CLONE_NEWPID | CLONE_NEWNET)))
CLONE_NEWPID | CLONE_NEWNET)))
return 0;

if (!capable(CAP_SYS_ADMIN)) {
Expand Down Expand Up @@ -173,8 +164,6 @@ void free_nsproxy(struct nsproxy *ns)
put_ipc_ns(ns->ipc_ns);
if (ns->pid_ns)
put_pid_ns(ns->pid_ns);
if (ns->user_ns)
put_user_ns(ns->user_ns);
put_net(ns->net_ns);
kmem_cache_free(nsproxy_cachep, ns);
}
Expand All @@ -189,7 +178,7 @@ int unshare_nsproxy_namespaces(unsigned long unshare_flags,
int err = 0;

if (!(unshare_flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
CLONE_NEWUSER | CLONE_NEWNET)))
CLONE_NEWNET)))
return 0;

if (!capable(CAP_SYS_ADMIN))
Expand Down
4 changes: 2 additions & 2 deletions kernel/sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -565,13 +565,13 @@ static int set_user(struct cred *new)
{
struct user_struct *new_user;

new_user = alloc_uid(current->nsproxy->user_ns, new->uid);
new_user = alloc_uid(current_user_ns(), new->uid);
if (!new_user)
return -EAGAIN;

if (atomic_read(&new_user->processes) >=
current->signal->rlim[RLIMIT_NPROC].rlim_cur &&
new_user != current->nsproxy->user_ns->root_user) {
new_user != INIT_USER) {
free_uid(new_user);
return -EAGAIN;
}
Expand Down
Loading

0 comments on commit 18b6e04

Please sign in to comment.