[PATCH v2 2/2] cred: change keyctl_session_to_parent() to usetask_work_queue()
From: Oleg Nesterov
Date: Wed Apr 11 2012 - 22:49:29 EST
Change keyctl_session_to_parent() to use task_work_queue() and
move key_replace_session_keyring() logic into task_work->func().
Note that we do task_work_cancel() before task_work_queue() to
ensure that only one work can be pending at any time. This is
important, we must not allow user-space to abuse the parent's
->task_works list.
The callback, replace_session_keyring(), checks PF_EXITING.
I guess this is not really needed but looks better.
We do not report the error if we race with the exiting parent
and task_work_queue() fails, this matches the current behaviour.
As a side effect, this fixes the (unlikely) race. The callers
of key_replace_session_keyring() and keyctl_session_to_parent()
lack the necessary barriers, the parent can miss the request.
Now we can remove task_struct->replacement_session_keyring and
related code.
Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
---
include/linux/key.h | 2 +-
security/keys/keyctl.c | 87 +++++++++++++++++++++++++++++++-----------
security/keys/process_keys.c | 49 -----------------------
3 files changed, 65 insertions(+), 73 deletions(-)
diff --git a/include/linux/key.h b/include/linux/key.h
index 96933b1..2f00c2a 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -303,7 +303,7 @@ static inline bool key_is_instantiated(const struct key *key)
extern ctl_table key_sysctls[];
#endif
-extern void key_replace_session_keyring(void);
+#define key_replace_session_keyring() do { } while (0)
/*
* the userspace interface
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index fb767c6..00b2e02 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <linux/vmalloc.h>
#include <linux/security.h>
+#include <linux/task_work.h>
#include <asm/uaccess.h>
#include "internal.h"
@@ -1409,6 +1410,53 @@ long keyctl_get_security(key_serial_t keyid,
return ret;
}
+static void replace_session_keyring(struct task_work *twork)
+{
+ const struct cred *old = current_cred();
+ struct cred *new = twork->data;
+
+ kfree(twork);
+ if (unlikely(current->flags & PF_EXITING)) {
+ put_cred(new);
+ return;
+ }
+
+ new-> uid = old-> uid;
+ new-> euid = old-> euid;
+ new-> suid = old-> suid;
+ new->fsuid = old->fsuid;
+ new-> gid = old-> gid;
+ new-> egid = old-> egid;
+ 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;
+ new->cap_inheritable = old->cap_inheritable;
+ new->cap_permitted = old->cap_permitted;
+ new->cap_effective = old->cap_effective;
+ new->cap_bset = old->cap_bset;
+
+ new->jit_keyring = old->jit_keyring;
+ new->thread_keyring = key_get(old->thread_keyring);
+ new->tgcred->tgid = old->tgcred->tgid;
+ new->tgcred->process_keyring = key_get(old->tgcred->process_keyring);
+
+ security_transfer_creds(new, old);
+
+ commit_creds(new);
+}
+
+static void free_cred_work(struct task_work *twork)
+{
+ if (twork) {
+ put_cred(twork->data);
+ kfree(twork);
+ }
+}
+
/*
* Attempt to install the calling process's session keyring on the process's
* parent process.
@@ -1423,27 +1471,31 @@ long keyctl_get_security(key_serial_t keyid,
*/
long keyctl_session_to_parent(void)
{
-#ifdef TIF_NOTIFY_RESUME
struct task_struct *me, *parent;
const struct cred *mycred, *pcred;
- struct cred *cred, *oldcred;
+ struct task_work *newwork, *oldwork;
key_ref_t keyring_r;
+ struct cred *cred;
int ret;
keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0, KEY_LINK);
if (IS_ERR(keyring_r))
return PTR_ERR(keyring_r);
+ ret = -ENOMEM;
+ newwork = kmalloc(sizeof(struct task_work), GFP_KERNEL);
+ if (!newwork)
+ goto error_keyring;
+
/* our parent is going to need a new cred struct, a new tgcred struct
* and new security data, so we allocate them here to prevent ENOMEM in
* our parent */
- ret = -ENOMEM;
cred = cred_alloc_blank();
if (!cred)
goto error_keyring;
cred->tgcred->session_keyring = key_ref_to_ptr(keyring_r);
- keyring_r = NULL;
+ init_task_work(newwork, replace_session_keyring, cred);
me = current;
rcu_read_lock();
@@ -1484,20 +1536,17 @@ long keyctl_session_to_parent(void)
mycred->tgcred->session_keyring->uid != mycred->euid)
goto not_permitted;
- /* if there's an already pending keyring replacement, then we replace
- * that */
- oldcred = parent->replacement_session_keyring;
+ /* cancel an already pending keyring replacement */
+ oldwork = task_work_cancel(parent, replace_session_keyring);
/* the replacement session keyring is applied just prior to userspace
* restarting */
- parent->replacement_session_keyring = cred;
- cred = NULL;
- set_ti_thread_flag(task_thread_info(parent), TIF_NOTIFY_RESUME);
-
+ if (!task_work_queue(parent, newwork))
+ newwork = NULL;
write_unlock_irq(&tasklist_lock);
rcu_read_unlock();
- if (oldcred)
- put_cred(oldcred);
+ free_cred_work(oldwork);
+ free_cred_work(newwork);
return 0;
already_same:
@@ -1505,21 +1554,13 @@ already_same:
not_permitted:
write_unlock_irq(&tasklist_lock);
rcu_read_unlock();
- put_cred(cred);
+ free_cred_work(newwork);
return ret;
error_keyring:
+ kfree(newwork);
key_ref_put(keyring_r);
return ret;
-
-#else /* !TIF_NOTIFY_RESUME */
- /*
- * To be removed when TIF_NOTIFY_RESUME has been implemented on
- * m68k/xtensa
- */
-#warning TIF_NOTIFY_RESUME not implemented
- return -EOPNOTSUPP;
-#endif /* !TIF_NOTIFY_RESUME */
}
/*
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index be7ecb2..6629137 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -827,52 +827,3 @@ error:
abort_creds(new);
return ret;
}
-
-/*
- * 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)
-{
- const struct cred *old;
- struct cred *new;
-
- if (!current->replacement_session_keyring)
- return;
-
- write_lock_irq(&tasklist_lock);
- new = current->replacement_session_keyring;
- current->replacement_session_keyring = NULL;
- write_unlock_irq(&tasklist_lock);
-
- if (!new)
- return;
-
- old = current_cred();
- new-> uid = old-> uid;
- new-> euid = old-> euid;
- new-> suid = old-> suid;
- new->fsuid = old->fsuid;
- new-> gid = old-> gid;
- new-> egid = old-> egid;
- 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;
- new->cap_inheritable = old->cap_inheritable;
- new->cap_permitted = old->cap_permitted;
- new->cap_effective = old->cap_effective;
- new->cap_bset = old->cap_bset;
-
- new->jit_keyring = old->jit_keyring;
- new->thread_keyring = key_get(old->thread_keyring);
- new->tgcred->tgid = old->tgcred->tgid;
- new->tgcred->process_keyring = key_get(old->tgcred->process_keyring);
-
- security_transfer_creds(new, old);
-
- commit_creds(new);
-}
--
1.5.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/