Skip to content

Commit

Permalink
KEYS: Make the session and process keyrings per-thread
Browse files Browse the repository at this point in the history 
Make the session keyring per-thread rather than per-process, but still
inherited from the parent thread to solve a problem with PAM and gdm.

The problem is that join_session_keyring() will reject attempts to change the
session keyring of a multithreaded program but gdm is now multithreaded before
it gets to the point of starting PAM and running pam_keyinit to create the
session keyring.  See:

	https://bugs.freedesktop.org/show_bug.cgi?id=49211

The reason that join_session_keyring() will only change the session keyring
under a single-threaded environment is that it's hard to alter the other
thread's credentials to effect the change in a multi-threaded program.  The
problems are such as:

 (1) How to prevent two threads both running join_session_keyring() from
     racing.

 (2) Another thread's credentials may not be modified directly by this process.

 (3) The number of threads is uncertain whilst we're not holding the
     appropriate spinlock, making preallocation slightly tricky.

 (4) We could use TIF_NOTIFY_RESUME and key_replace_session_keyring() to get
     another thread to replace its keyring, but that means preallocating for
     each thread.

A reasonable way around this is to make the session keyring per-thread rather
than per-process and just document that if you want a common session keyring,
you must get it before you spawn any threads - which is the current situation
anyway.

Whilst we're at it, we can the process keyring behave in the same way.  This
means we can clean up some of the ickyness in the creds code.

Basically, after this patch, the session, process and thread keyrings are about
inheritance rules only and not about sharing changes of keyring.

Reported-by: Mantas M. <[email protected]>
Signed-off-by: David Howells <[email protected]>
Tested-by: Ray Strode <[email protected]>
  • Loading branch information
@dhowells
dhowells committed Oct 2, 2012
1 parent a84a921 commit 3a50597
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 181 deletions.
17 changes: 2 additions & 15 deletions include/linux/cred.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,6 @@ extern int groups_search(const struct group_info *, kgid_t);
extern int in_group_p(kgid_t);
extern int in_egroup_p(kgid_t);

/*
* The common credentials for a thread group
* - shared by CLONE_THREAD
*/
#ifdef CONFIG_KEYS
struct thread_group_cred {
atomic_t usage;
pid_t tgid; /* thread group process ID */
spinlock_t lock;
struct key __rcu *session_keyring; /* keyring inherited over fork */
struct key *process_keyring; /* keyring private to this process */
struct rcu_head rcu; /* RCU deletion hook */
};
#endif

/*
* The security context of a task
*
Expand Down Expand Up @@ -139,6 +124,8 @@ struct cred {
#ifdef CONFIG_KEYS
unsigned char jit_keyring; /* default keyring to attach requested
* keys to */
struct key __rcu *session_keyring; /* keyring inherited over fork */
struct key *process_keyring; /* keyring private to this process */
struct key *thread_keyring; /* keyring private to this thread */
struct key *request_key_auth; /* assumed request_key authority */
struct thread_group_cred *tgcred; /* thread-group shared credentials */
Expand Down
127 changes: 15 additions & 112 deletions kernel/cred.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,6 @@

static struct kmem_cache *cred_jar;

/*
* The common credentials for the initial task's thread group
*/
#ifdef CONFIG_KEYS
static struct thread_group_cred init_tgcred = {
.usage = ATOMIC_INIT(2),
.tgid = 0,
.lock = __SPIN_LOCK_UNLOCKED(init_cred.tgcred.lock),
};
#endif

/*
* The initial credentials for the initial task
*/
Expand All @@ -65,9 +54,6 @@ struct cred init_cred = {
.user = INIT_USER,
.user_ns = &init_user_ns,
.group_info = &init_groups,
#ifdef CONFIG_KEYS
.tgcred = &init_tgcred,
#endif
};

static inline void set_cred_subscribers(struct cred *cred, int n)
Expand Down Expand Up @@ -95,36 +81,6 @@ static inline void alter_cred_subscribers(const struct cred *_cred, int n)
#endif
}

/*
* Dispose of the shared task group credentials
*/
#ifdef CONFIG_KEYS
static void release_tgcred_rcu(struct rcu_head *rcu)
{
struct thread_group_cred *tgcred =
container_of(rcu, struct thread_group_cred, rcu);

BUG_ON(atomic_read(&tgcred->usage) != 0);

key_put(tgcred->session_keyring);
key_put(tgcred->process_keyring);
kfree(tgcred);
}
#endif

/*
* Release a set of thread group credentials.
*/
static void release_tgcred(struct cred *cred)
{
#ifdef CONFIG_KEYS
struct thread_group_cred *tgcred = cred->tgcred;

if (atomic_dec_and_test(&tgcred->usage))
call_rcu(&tgcred->rcu, release_tgcred_rcu);
#endif
}

/*
* The RCU callback to actually dispose of a set of credentials
*/
Expand All @@ -150,9 +106,10 @@ static void put_cred_rcu(struct rcu_head *rcu)
#endif

security_cred_free(cred);
key_put(cred->session_keyring);
key_put(cred->process_keyring);
key_put(cred->thread_keyring);
key_put(cred->request_key_auth);
release_tgcred(cred);
if (cred->group_info)
put_group_info(cred->group_info);
free_uid(cred->user);
Expand Down Expand Up @@ -246,15 +203,6 @@ struct cred *cred_alloc_blank(void)
if (!new)
return NULL;

#ifdef CONFIG_KEYS
new->tgcred = kzalloc(sizeof(*new->tgcred), GFP_KERNEL);
if (!new->tgcred) {
kmem_cache_free(cred_jar, new);
return NULL;
}
atomic_set(&new->tgcred->usage, 1);
#endif

atomic_set(&new->usage, 1);
#ifdef CONFIG_DEBUG_CREDENTIALS
new->magic = CRED_MAGIC;
Expand Down Expand Up @@ -308,9 +256,10 @@ struct cred *prepare_creds(void)
get_user_ns(new->user_ns);

#ifdef CONFIG_KEYS
key_get(new->session_keyring);
key_get(new->process_keyring);
key_get(new->thread_keyring);
key_get(new->request_key_auth);
atomic_inc(&new->tgcred->usage);
#endif

#ifdef CONFIG_SECURITY
Expand All @@ -334,39 +283,20 @@ EXPORT_SYMBOL(prepare_creds);
*/
struct cred *prepare_exec_creds(void)
{
struct thread_group_cred *tgcred = NULL;
struct cred *new;

#ifdef CONFIG_KEYS
tgcred = kmalloc(sizeof(*tgcred), GFP_KERNEL);
if (!tgcred)
return NULL;
#endif

new = prepare_creds();
if (!new) {
kfree(tgcred);
if (!new)
return new;
}

#ifdef CONFIG_KEYS
/* newly exec'd tasks don't get a thread keyring */
key_put(new->thread_keyring);
new->thread_keyring = NULL;

/* create a new per-thread-group creds for all this set of threads to
* share */
memcpy(tgcred, new->tgcred, sizeof(struct thread_group_cred));

atomic_set(&tgcred->usage, 1);
spin_lock_init(&tgcred->lock);

/* inherit the session keyring; new process keyring */
key_get(tgcred->session_keyring);
tgcred->process_keyring = NULL;

release_tgcred(new);
new->tgcred = tgcred;
key_put(new->process_keyring);
new->process_keyring = NULL;
#endif

return new;
Expand All @@ -383,9 +313,6 @@ struct cred *prepare_exec_creds(void)
*/
int copy_creds(struct task_struct *p, unsigned long clone_flags)
{
#ifdef CONFIG_KEYS
struct thread_group_cred *tgcred;
#endif
struct cred *new;
int ret;

Expand Down Expand Up @@ -425,22 +352,12 @@ int copy_creds(struct task_struct *p, unsigned long clone_flags)
install_thread_keyring_to_cred(new);
}

/* we share the process and session keyrings between all the threads in
* a process - this is slightly icky as we violate COW credentials a
* bit */
/* The process keyring is only shared between the threads in a process;
* anything outside of those threads doesn't inherit.
*/
if (!(clone_flags & CLONE_THREAD)) {
tgcred = kmalloc(sizeof(*tgcred), GFP_KERNEL);
if (!tgcred) {
ret = -ENOMEM;
goto error_put;
}
atomic_set(&tgcred->usage, 1);
spin_lock_init(&tgcred->lock);
tgcred->process_keyring = NULL;
tgcred->session_keyring = key_get(new->tgcred->session_keyring);

release_tgcred(new);
new->tgcred = tgcred;
key_put(new->process_keyring);
new->process_keyring = NULL;
}
#endif

Expand Down Expand Up @@ -643,24 +560,13 @@ void __init cred_init(void)
*/
struct cred *prepare_kernel_cred(struct task_struct *daemon)
{
#ifdef CONFIG_KEYS
struct thread_group_cred *tgcred;
#endif
const struct cred *old;
struct cred *new;

new = kmem_cache_alloc(cred_jar, GFP_KERNEL);
if (!new)
return NULL;

#ifdef CONFIG_KEYS
tgcred = kmalloc(sizeof(*tgcred), GFP_KERNEL);
if (!tgcred) {
kmem_cache_free(cred_jar, new);
return NULL;
}
#endif

kdebug("prepare_kernel_cred() alloc %p", new);

if (daemon)
Expand All @@ -678,13 +584,10 @@ struct cred *prepare_kernel_cred(struct task_struct *daemon)
get_group_info(new->group_info);

#ifdef CONFIG_KEYS
atomic_set(&tgcred->usage, 1);
spin_lock_init(&tgcred->lock);
tgcred->process_keyring = NULL;
tgcred->session_keyring = NULL;
new->tgcred = tgcred;
new->request_key_auth = NULL;
new->session_keyring = NULL;
new->process_keyring = NULL;
new->thread_keyring = NULL;
new->request_key_auth = NULL;
new->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
#endif

Expand Down
11 changes: 6 additions & 5 deletions security/keys/keyctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1475,7 +1475,8 @@ long keyctl_session_to_parent(void)
goto error_keyring;
newwork = &cred->rcu;

cred->tgcred->session_keyring = key_ref_to_ptr(keyring_r);
cred->session_keyring = key_ref_to_ptr(keyring_r);
keyring_r = NULL;
init_task_work(newwork, key_change_session_keyring);

me = current;
Expand All @@ -1500,7 +1501,7 @@ long keyctl_session_to_parent(void)
mycred = current_cred();
pcred = __task_cred(parent);
if (mycred == pcred ||
mycred->tgcred->session_keyring == pcred->tgcred->session_keyring) {
mycred->session_keyring == pcred->session_keyring) {
ret = 0;
goto unlock;
}
Expand All @@ -1516,9 +1517,9 @@ long keyctl_session_to_parent(void)
goto unlock;

/* the keyrings must have the same UID */
if ((pcred->tgcred->session_keyring &&
pcred->tgcred->session_keyring->uid != mycred->euid) ||
mycred->tgcred->session_keyring->uid != mycred->euid)
if ((pcred->session_keyring &&
pcred->session_keyring->uid != mycred->euid) ||
mycred->session_keyring->uid != mycred->euid)
goto unlock;

/* cancel an already pending keyring replacement */
Expand Down
Loading

0 comments on commit 3a50597

Please sign in to comment.