From 27d6379894be4a81984da4d48002196a83939ca9 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Thu, 28 Oct 2010 13:16:13 +0100 Subject: Fix install_process_keyring error handling Fix an incorrect error check that returns 1 for error instead of the expected error code. Signed-off-by: Andi Kleen Signed-off-by: David Howells Signed-off-by: Linus Torvalds --- security/keys/process_keys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/keys/process_keys.c') diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index f8e7251ae2c8..504bdd2452bd 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -207,7 +207,7 @@ static int install_process_keyring(void) ret = install_process_keyring_to_cred(new); if (ret < 0) { abort_creds(new); - return ret != -EEXIST ?: 0; + return ret != -EEXIST ? ret : 0; } return commit_creds(new); -- cgit v1.2.2 From a8b17ed019bd40d3bfa20439d9c36a99f9be9180 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 20 Jan 2011 16:38:27 +0000 Subject: KEYS: Do some style cleanup in the key management code. Do a bit of a style clean up in the key management code. No functional changes. Done using: perl -p -i -e 's!^/[*]*/\n!!' security/keys/*.c perl -p -i -e 's!} /[*] end [a-z0-9_]*[(][)] [*]/\n!}\n!' security/keys/*.c sed -i -s -e ": next" -e N -e 's/^\n[}]$/}/' -e t -e P -e 's/^.*\n//' -e "b next" security/keys/*.c To remove /*****/ lines, remove comments on the closing brace of a function to name the function and remove blank lines before the closing brace of a function. Signed-off-by: David Howells Signed-off-by: Linus Torvalds --- security/keys/process_keys.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) (limited to 'security/keys/process_keys.c') diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 504bdd2452bd..ea55cf9acf36 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -38,7 +38,6 @@ struct key_user root_key_user = { .user_ns = &init_user_ns, }; -/*****************************************************************************/ /* * install user and user session keyrings for a particular UID */ @@ -275,7 +274,6 @@ static int install_session_keyring(struct key *keyring) return commit_creds(new); } -/*****************************************************************************/ /* * the filesystem user ID changed */ @@ -288,10 +286,8 @@ void key_fsuid_changed(struct task_struct *tsk) tsk->cred->thread_keyring->uid = tsk->cred->fsuid; up_write(&tsk->cred->thread_keyring->sem); } +} -} /* end key_fsuid_changed() */ - -/*****************************************************************************/ /* * the filesystem group ID changed */ @@ -304,10 +300,8 @@ void key_fsgid_changed(struct task_struct *tsk) tsk->cred->thread_keyring->gid = tsk->cred->fsgid; up_write(&tsk->cred->thread_keyring->sem); } +} -} /* end key_fsgid_changed() */ - -/*****************************************************************************/ /* * search only my process keyrings for the first matching key * - we use the supplied match function to see if the description (or other @@ -428,7 +422,6 @@ found: return key_ref; } -/*****************************************************************************/ /* * search the process keyrings for the first matching key * - we use the supplied match function to see if the description (or other @@ -489,20 +482,16 @@ key_ref_t search_process_keyrings(struct key_type *type, found: return key_ref; +} -} /* end search_process_keyrings() */ - -/*****************************************************************************/ /* * see if the key we're looking at is the target key */ int lookup_user_key_possessed(const struct key *key, const void *target) { return key == target; +} -} /* end lookup_user_key_possessed() */ - -/*****************************************************************************/ /* * lookup a key given a key ID from userspace with a given permissions mask * - don't create special keyrings unless so requested @@ -711,10 +700,8 @@ invalid_key: reget_creds: put_cred(cred); goto try_again; +} -} /* end lookup_user_key() */ - -/*****************************************************************************/ /* * join the named keyring as the session keyring if possible, or attempt to * create a new one of that name if not -- cgit v1.2.2 From 973c9f4f49ca96a53bcf6384c4c59ccd26c33906 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 20 Jan 2011 16:38:33 +0000 Subject: KEYS: Fix up comments in key management code Fix up comments in the key management code. No functional changes. Signed-off-by: David Howells Signed-off-by: Linus Torvalds --- security/keys/process_keys.c | 112 +++++++++++++++++++++++++++++-------------- 1 file changed, 75 insertions(+), 37 deletions(-) (limited to 'security/keys/process_keys.c') diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index ea55cf9acf36..930634e45149 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -1,4 +1,4 @@ -/* Management of a process's keyrings +/* Manage a process's keyrings * * Copyright (C) 2004-2005, 2008 Red Hat, Inc. All Rights Reserved. * Written by David Howells (dhowells@redhat.com) @@ -21,13 +21,13 @@ #include #include "internal.h" -/* session keyring create vs join semaphore */ +/* Session keyring create vs join semaphore */ static DEFINE_MUTEX(key_session_mutex); -/* user keyring creation semaphore */ +/* User keyring creation semaphore */ static DEFINE_MUTEX(key_user_keyring_mutex); -/* the root user's tracking struct */ +/* The root user's tracking struct */ struct key_user root_key_user = { .usage = ATOMIC_INIT(3), .cons_lock = __MUTEX_INITIALIZER(root_key_user.cons_lock), @@ -39,7 +39,7 @@ struct key_user root_key_user = { }; /* - * install user and user session keyrings for a particular UID + * Install the user and user session keyrings for the current process's UID. */ int install_user_keyrings(void) { @@ -121,7 +121,8 @@ error: } /* - * install a fresh thread keyring directly to new credentials + * Install a fresh thread keyring directly to new credentials. This keyring is + * allowed to overrun the quota. */ int install_thread_keyring_to_cred(struct cred *new) { @@ -137,7 +138,7 @@ int install_thread_keyring_to_cred(struct cred *new) } /* - * install a fresh thread keyring, discarding the old one + * Install a fresh thread keyring, discarding the old one. */ static int install_thread_keyring(void) { @@ -160,9 +161,10 @@ static int install_thread_keyring(void) } /* - * install a process keyring directly to a credentials struct - * - returns -EEXIST if there was already a process keyring, 0 if one installed, - * and other -ve on any other error + * Install a process keyring directly to a credentials struct. + * + * Returns -EEXIST if there was already a process keyring, 0 if one installed, + * and other value on any other error */ int install_process_keyring_to_cred(struct cred *new) { @@ -191,8 +193,11 @@ int install_process_keyring_to_cred(struct cred *new) } /* - * make sure a process keyring is installed - * - we + * Make sure a process keyring is installed for the current process. The + * existing process keyring is not replaced. + * + * Returns 0 if there is a process keyring by the end of this function, some + * error otherwise. */ static int install_process_keyring(void) { @@ -213,7 +218,7 @@ static int install_process_keyring(void) } /* - * install a session keyring directly to a credentials struct + * Install a session keyring directly to a credentials struct. */ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring) { @@ -253,8 +258,8 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring) } /* - * install a session keyring, discarding the old one - * - if a keyring is not supplied, an empty one is invented + * Install a session keyring, discarding the old one. If a keyring is not + * supplied, an empty one is invented. */ static int install_session_keyring(struct key *keyring) { @@ -275,7 +280,7 @@ static int install_session_keyring(struct key *keyring) } /* - * the filesystem user ID changed + * Handle the fsuid changing. */ void key_fsuid_changed(struct task_struct *tsk) { @@ -289,7 +294,7 @@ void key_fsuid_changed(struct task_struct *tsk) } /* - * the filesystem group ID changed + * Handle the fsgid changing. */ void key_fsgid_changed(struct task_struct *tsk) { @@ -303,11 +308,25 @@ void key_fsgid_changed(struct task_struct *tsk) } /* - * search only my process keyrings for the first matching key - * - we use the supplied match function to see if the description (or other - * feature of interest) matches - * - we return -EAGAIN if we didn't find any matching key - * - we return -ENOKEY if we found only negative matching keys + * Search the process keyrings attached to the supplied cred for the first + * matching key. + * + * The search criteria are the type and the match function. The description is + * given to the match function as a parameter, but doesn't otherwise influence + * the search. Typically the match function will compare the description + * parameter to the key's description. + * + * This can only search keyrings that grant Search permission to the supplied + * credentials. Keyrings linked to searched keyrings will also be searched if + * they grant Search permission too. Keys can only be found if they grant + * Search permission to the credentials. + * + * Returns a pointer to the key with the key usage count incremented if + * successful, -EAGAIN if we didn't find any matching key or -ENOKEY if we only + * matched negative keys. + * + * In the case of a successful return, the possession attribute is set on the + * returned key reference. */ key_ref_t search_my_process_keyrings(struct key_type *type, const void *description, @@ -423,11 +442,12 @@ found: } /* - * search the process keyrings for the first matching key - * - we use the supplied match function to see if the description (or other - * feature of interest) matches - * - we return -EAGAIN if we didn't find any matching key - * - we return -ENOKEY if we found only negative matching keys + * Search the process keyrings attached to the supplied cred for the first + * matching key in the manner of search_my_process_keyrings(), but also search + * the keys attached to the assumed authorisation key using its credentials if + * one is available. + * + * Return same as search_my_process_keyrings(). */ key_ref_t search_process_keyrings(struct key_type *type, const void *description, @@ -485,7 +505,7 @@ found: } /* - * see if the key we're looking at is the target key + * See if the key we're looking at is the target key. */ int lookup_user_key_possessed(const struct key *key, const void *target) { @@ -493,9 +513,22 @@ int lookup_user_key_possessed(const struct key *key, const void *target) } /* - * lookup a key given a key ID from userspace with a given permissions mask - * - don't create special keyrings unless so requested - * - partially constructed keys aren't found unless requested + * Look up a key ID given us by userspace with a given permissions mask to get + * the key it refers to. + * + * Flags can be passed to request that special keyrings be created if referred + * to directly, to permit partially constructed keys to be found and to skip + * validity and permission checks on the found key. + * + * Returns a pointer to the key with an incremented usage count if successful; + * -EINVAL if the key ID is invalid; -ENOKEY if the key ID does not correspond + * to a key or the best found key was a negative key; -EKEYREVOKED or + * -EKEYEXPIRED if the best found key was revoked or expired; -EACCES if the + * found key doesn't grant the requested permit or the LSM denied access to it; + * or -ENOMEM if a special keyring couldn't be created. + * + * In the case of a successful return, the possession attribute is set on the + * returned key reference. */ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags, key_perm_t perm) @@ -703,10 +736,15 @@ reget_creds: } /* - * join the named keyring as the session keyring if possible, or attempt to - * create a new one of that name if not - * - if the name is NULL, an empty anonymous keyring is installed instead - * - named session keyring joining is done with a semaphore held + * Join the named keyring as the session keyring if possible else attempt to + * create a new one of that name and join that. + * + * If the name is NULL, an empty anonymous keyring will be installed as the + * session keyring. + * + * Named session keyrings are joined with a semaphore held to prevent the + * keyrings from going away whilst the attempt is made to going them and also + * to prevent a race in creating compatible session keyrings. */ long join_session_keyring(const char *name) { @@ -778,8 +816,8 @@ error: } /* - * Replace a process's session keyring when that process resumes userspace on - * behalf of one of its children + * Replace a process's session keyring on behalf of one of its children when + * the target process is about to resume userspace execution. */ void key_replace_session_keyring(void) { -- cgit v1.2.2 From 78b7280cce23293f7570ad52c1ffe1485c6d9669 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 11 Mar 2011 17:57:23 +0000 Subject: KEYS: Improve /proc/keys Improve /proc/keys by: (1) Don't attempt to summarise the payload of a negated key. It won't have one. To this end, a helper function - key_is_instantiated() has been added that allows the caller to find out whether the key is positively instantiated (as opposed to being uninstantiated or negatively instantiated). (2) Do show keys that are negative, expired or revoked rather than hiding them. This requires an override flag (no_state_check) to be passed to search_my_process_keyrings() and keyring_search_aux() to suppress this check. Without this, keys that are possessed by the caller, but only grant permissions to the caller if possessed are skipped as the possession check fails. Keys that are visible due to user, group or other checks are visible with or without this patch. Signed-off-by: David Howells Signed-off-by: James Morris --- security/keys/process_keys.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'security/keys/process_keys.c') diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 930634e45149..6c0480db8885 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -331,6 +331,7 @@ void key_fsgid_changed(struct task_struct *tsk) key_ref_t search_my_process_keyrings(struct key_type *type, const void *description, key_match_func_t match, + bool no_state_check, const struct cred *cred) { key_ref_t key_ref, ret, err; @@ -350,7 +351,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, if (cred->thread_keyring) { key_ref = keyring_search_aux( make_key_ref(cred->thread_keyring, 1), - cred, type, description, match); + cred, type, description, match, no_state_check); if (!IS_ERR(key_ref)) goto found; @@ -371,7 +372,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, if (cred->tgcred->process_keyring) { key_ref = keyring_search_aux( make_key_ref(cred->tgcred->process_keyring, 1), - cred, type, description, match); + cred, type, description, match, no_state_check); if (!IS_ERR(key_ref)) goto found; @@ -395,7 +396,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, make_key_ref(rcu_dereference( cred->tgcred->session_keyring), 1), - cred, type, description, match); + cred, type, description, match, no_state_check); rcu_read_unlock(); if (!IS_ERR(key_ref)) @@ -417,7 +418,7 @@ key_ref_t search_my_process_keyrings(struct key_type *type, else if (cred->user->session_keyring) { key_ref = keyring_search_aux( make_key_ref(cred->user->session_keyring, 1), - cred, type, description, match); + cred, type, description, match, no_state_check); if (!IS_ERR(key_ref)) goto found; @@ -459,7 +460,8 @@ key_ref_t search_process_keyrings(struct key_type *type, might_sleep(); - key_ref = search_my_process_keyrings(type, description, match, cred); + key_ref = search_my_process_keyrings(type, description, match, + false, cred); if (!IS_ERR(key_ref)) goto found; err = key_ref; -- cgit v1.2.2 From f7285b5d631fd6096b11c6af0058ed3a2b30ef4e Mon Sep 17 00:00:00 2001 From: "Serge E. Hallyn" Date: Thu, 26 May 2011 15:25:05 -0500 Subject: Set cred->user_ns in key_replace_session_keyring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since this cred was not created with copy_creds(), it needs to get initialized. Otherwise use of syscall(__NR_keyctl, KEYCTL_SESSION_TO_PARENT); can lead to a NULL deref. Thanks to Robert for finding this. But introduced by commit 47a150edc2a ("Cache user_ns in struct cred"). Signed-off-by: Serge E. Hallyn Reported-by: Robert Święcki Cc: David Howells Cc: stable@kernel.org (2.6.39) Signed-off-by: Linus Torvalds --- security/keys/process_keys.c | 1 + 1 file changed, 1 insertion(+) (limited to 'security/keys/process_keys.c') diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 6c0480db8885..a3063eb3dc23 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -847,6 +847,7 @@ void key_replace_session_keyring(void) new-> sgid = old-> sgid; new->fsgid = old->fsgid; new->user = get_uid(old->user); + new->user_ns = new->user->user_ns; new->group_info = get_group_info(old->group_info); new->securebits = old->securebits; -- cgit v1.2.2