diff options
author | Oleg Nesterov <oleg@redhat.com> | 2012-05-10 20:59:08 -0400 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2012-05-23 22:11:23 -0400 |
commit | 413cd3d9abeaef590e5ce00564f7a443165db238 (patch) | |
tree | fc7d254053793a95d1470f7c9eafb782d8cf91d6 /security/keys | |
parent | 4d1d61a6b203d957777d73fcebf19d90b038b5b2 (diff) |
keys: change keyctl_session_to_parent() to use task_work_add()
Change keyctl_session_to_parent() to use task_work_add() and move
key_replace_session_keyring() logic into task_work->func().
Note that we do task_work_cancel() before task_work_add() 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.
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@redhat.com>
Acked-by: David Howells <dhowells@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Richard Kuo <rkuo@codeaurora.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Alexander Gordeev <agordeev@redhat.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: David Smith <dsmith@redhat.com>
Cc: "Frank Ch. Eigler" <fche@redhat.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Larry Woodman <lwoodman@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Diffstat (limited to 'security/keys')
-rw-r--r-- | security/keys/internal.h | 2 | ||||
-rw-r--r-- | security/keys/keyctl.c | 63 | ||||
-rw-r--r-- | security/keys/process_keys.c | 20 |
3 files changed, 44 insertions, 41 deletions
diff --git a/security/keys/internal.h b/security/keys/internal.h index f711b094ed41..3dcbf86b0d31 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h | |||
@@ -14,6 +14,7 @@ | |||
14 | 14 | ||
15 | #include <linux/sched.h> | 15 | #include <linux/sched.h> |
16 | #include <linux/key-type.h> | 16 | #include <linux/key-type.h> |
17 | #include <linux/task_work.h> | ||
17 | 18 | ||
18 | #ifdef __KDEBUG | 19 | #ifdef __KDEBUG |
19 | #define kenter(FMT, ...) \ | 20 | #define kenter(FMT, ...) \ |
@@ -148,6 +149,7 @@ extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags, | |||
148 | #define KEY_LOOKUP_FOR_UNLINK 0x04 | 149 | #define KEY_LOOKUP_FOR_UNLINK 0x04 |
149 | 150 | ||
150 | extern long join_session_keyring(const char *name); | 151 | extern long join_session_keyring(const char *name); |
152 | extern void key_change_session_keyring(struct task_work *twork); | ||
151 | 153 | ||
152 | extern struct work_struct key_gc_work; | 154 | extern struct work_struct key_gc_work; |
153 | extern unsigned key_gc_delay; | 155 | extern unsigned key_gc_delay; |
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 534a634283a4..2f28126215a2 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c | |||
@@ -1456,47 +1456,55 @@ long keyctl_session_to_parent(void) | |||
1456 | { | 1456 | { |
1457 | struct task_struct *me, *parent; | 1457 | struct task_struct *me, *parent; |
1458 | const struct cred *mycred, *pcred; | 1458 | const struct cred *mycred, *pcred; |
1459 | struct cred *cred, *oldcred; | 1459 | struct task_work *newwork, *oldwork; |
1460 | key_ref_t keyring_r; | 1460 | key_ref_t keyring_r; |
1461 | struct cred *cred; | ||
1461 | int ret; | 1462 | int ret; |
1462 | 1463 | ||
1463 | keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0, KEY_LINK); | 1464 | keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0, KEY_LINK); |
1464 | if (IS_ERR(keyring_r)) | 1465 | if (IS_ERR(keyring_r)) |
1465 | return PTR_ERR(keyring_r); | 1466 | return PTR_ERR(keyring_r); |
1466 | 1467 | ||
1468 | ret = -ENOMEM; | ||
1469 | newwork = kmalloc(sizeof(struct task_work), GFP_KERNEL); | ||
1470 | if (!newwork) | ||
1471 | goto error_keyring; | ||
1472 | |||
1467 | /* our parent is going to need a new cred struct, a new tgcred struct | 1473 | /* our parent is going to need a new cred struct, a new tgcred struct |
1468 | * and new security data, so we allocate them here to prevent ENOMEM in | 1474 | * and new security data, so we allocate them here to prevent ENOMEM in |
1469 | * our parent */ | 1475 | * our parent */ |
1470 | ret = -ENOMEM; | ||
1471 | cred = cred_alloc_blank(); | 1476 | cred = cred_alloc_blank(); |
1472 | if (!cred) | 1477 | if (!cred) |
1473 | goto error_keyring; | 1478 | goto error_newwork; |
1474 | 1479 | ||
1475 | cred->tgcred->session_keyring = key_ref_to_ptr(keyring_r); | 1480 | cred->tgcred->session_keyring = key_ref_to_ptr(keyring_r); |
1476 | keyring_r = NULL; | 1481 | init_task_work(newwork, key_change_session_keyring, cred); |
1477 | 1482 | ||
1478 | me = current; | 1483 | me = current; |
1479 | rcu_read_lock(); | 1484 | rcu_read_lock(); |
1480 | write_lock_irq(&tasklist_lock); | 1485 | write_lock_irq(&tasklist_lock); |
1481 | 1486 | ||
1482 | parent = me->real_parent; | ||
1483 | ret = -EPERM; | 1487 | ret = -EPERM; |
1488 | oldwork = NULL; | ||
1489 | parent = me->real_parent; | ||
1484 | 1490 | ||
1485 | /* the parent mustn't be init and mustn't be a kernel thread */ | 1491 | /* the parent mustn't be init and mustn't be a kernel thread */ |
1486 | if (parent->pid <= 1 || !parent->mm) | 1492 | if (parent->pid <= 1 || !parent->mm) |
1487 | goto not_permitted; | 1493 | goto unlock; |
1488 | 1494 | ||
1489 | /* the parent must be single threaded */ | 1495 | /* the parent must be single threaded */ |
1490 | if (!thread_group_empty(parent)) | 1496 | if (!thread_group_empty(parent)) |
1491 | goto not_permitted; | 1497 | goto unlock; |
1492 | 1498 | ||
1493 | /* the parent and the child must have different session keyrings or | 1499 | /* the parent and the child must have different session keyrings or |
1494 | * there's no point */ | 1500 | * there's no point */ |
1495 | mycred = current_cred(); | 1501 | mycred = current_cred(); |
1496 | pcred = __task_cred(parent); | 1502 | pcred = __task_cred(parent); |
1497 | if (mycred == pcred || | 1503 | if (mycred == pcred || |
1498 | mycred->tgcred->session_keyring == pcred->tgcred->session_keyring) | 1504 | mycred->tgcred->session_keyring == pcred->tgcred->session_keyring) { |
1499 | goto already_same; | 1505 | ret = 0; |
1506 | goto unlock; | ||
1507 | } | ||
1500 | 1508 | ||
1501 | /* the parent must have the same effective ownership and mustn't be | 1509 | /* the parent must have the same effective ownership and mustn't be |
1502 | * SUID/SGID */ | 1510 | * SUID/SGID */ |
@@ -1506,38 +1514,37 @@ long keyctl_session_to_parent(void) | |||
1506 | pcred->gid != mycred->egid || | 1514 | pcred->gid != mycred->egid || |
1507 | pcred->egid != mycred->egid || | 1515 | pcred->egid != mycred->egid || |
1508 | pcred->sgid != mycred->egid) | 1516 | pcred->sgid != mycred->egid) |
1509 | goto not_permitted; | 1517 | goto unlock; |
1510 | 1518 | ||
1511 | /* the keyrings must have the same UID */ | 1519 | /* the keyrings must have the same UID */ |
1512 | if ((pcred->tgcred->session_keyring && | 1520 | if ((pcred->tgcred->session_keyring && |
1513 | pcred->tgcred->session_keyring->uid != mycred->euid) || | 1521 | pcred->tgcred->session_keyring->uid != mycred->euid) || |
1514 | mycred->tgcred->session_keyring->uid != mycred->euid) | 1522 | mycred->tgcred->session_keyring->uid != mycred->euid) |
1515 | goto not_permitted; | 1523 | goto unlock; |
1516 | 1524 | ||
1517 | /* if there's an already pending keyring replacement, then we replace | 1525 | /* cancel an already pending keyring replacement */ |
1518 | * that */ | 1526 | oldwork = task_work_cancel(parent, key_change_session_keyring); |
1519 | oldcred = parent->replacement_session_keyring; | ||
1520 | 1527 | ||
1521 | /* the replacement session keyring is applied just prior to userspace | 1528 | /* the replacement session keyring is applied just prior to userspace |
1522 | * restarting */ | 1529 | * restarting */ |
1523 | parent->replacement_session_keyring = cred; | 1530 | ret = task_work_add(parent, newwork, true); |
1524 | cred = NULL; | 1531 | if (!ret) |
1525 | set_ti_thread_flag(task_thread_info(parent), TIF_NOTIFY_RESUME); | 1532 | newwork = NULL; |
1526 | 1533 | unlock: | |
1527 | write_unlock_irq(&tasklist_lock); | ||
1528 | rcu_read_unlock(); | ||
1529 | if (oldcred) | ||
1530 | put_cred(oldcred); | ||
1531 | return 0; | ||
1532 | |||
1533 | already_same: | ||
1534 | ret = 0; | ||
1535 | not_permitted: | ||
1536 | write_unlock_irq(&tasklist_lock); | 1534 | write_unlock_irq(&tasklist_lock); |
1537 | rcu_read_unlock(); | 1535 | rcu_read_unlock(); |
1538 | put_cred(cred); | 1536 | if (oldwork) { |
1537 | put_cred(oldwork->data); | ||
1538 | kfree(oldwork); | ||
1539 | } | ||
1540 | if (newwork) { | ||
1541 | put_cred(newwork->data); | ||
1542 | kfree(newwork); | ||
1543 | } | ||
1539 | return ret; | 1544 | return ret; |
1540 | 1545 | ||
1546 | error_newwork: | ||
1547 | kfree(newwork); | ||
1541 | error_keyring: | 1548 | error_keyring: |
1542 | key_ref_put(keyring_r); | 1549 | key_ref_put(keyring_r); |
1543 | return ret; | 1550 | return ret; |
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index d71056db7b67..4ad54eea1ea4 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c | |||
@@ -834,23 +834,17 @@ error: | |||
834 | * Replace a process's session keyring on behalf of one of its children when | 834 | * Replace a process's session keyring on behalf of one of its children when |
835 | * the target process is about to resume userspace execution. | 835 | * the target process is about to resume userspace execution. |
836 | */ | 836 | */ |
837 | void key_replace_session_keyring(void) | 837 | void key_change_session_keyring(struct task_work *twork) |
838 | { | 838 | { |
839 | const struct cred *old; | 839 | const struct cred *old = current_cred(); |
840 | struct cred *new; | 840 | struct cred *new = twork->data; |
841 | |||
842 | if (!current->replacement_session_keyring) | ||
843 | return; | ||
844 | 841 | ||
845 | write_lock_irq(&tasklist_lock); | 842 | kfree(twork); |
846 | new = current->replacement_session_keyring; | 843 | if (unlikely(current->flags & PF_EXITING)) { |
847 | current->replacement_session_keyring = NULL; | 844 | put_cred(new); |
848 | write_unlock_irq(&tasklist_lock); | ||
849 | |||
850 | if (!new) | ||
851 | return; | 845 | return; |
846 | } | ||
852 | 847 | ||
853 | old = current_cred(); | ||
854 | new-> uid = old-> uid; | 848 | new-> uid = old-> uid; |
855 | new-> euid = old-> euid; | 849 | new-> euid = old-> euid; |
856 | new-> suid = old-> suid; | 850 | new-> suid = old-> suid; |