Skip to content

Commit

Permalink
[PATCH] keys: Discard key spinlock and use RCU for key payload
Browse files Browse the repository at this point in the history 
The attached patch changes the key implementation in a number of ways:

 (1) It removes the spinlock from the key structure.

 (2) The key flags are now accessed using atomic bitops instead of
     write-locking the key spinlock and using C bitwise operators.

     The three instantiation flags are dealt with with the construction
     semaphore held during the request_key/instantiate/negate sequence, thus
     rendering the spinlock superfluous.

     The key flags are also now bit numbers not bit masks.

 (3) The key payload is now accessed using RCU. This permits the recursive
     keyring search algorithm to be simplified greatly since no locks need be
     taken other than the usual RCU preemption disablement. Searching now does
     not require any locks or semaphores to be held; merely that the starting
     keyring be pinned.

 (4) The keyring payload now includes an RCU head so that it can be disposed
     of by call_rcu(). This requires that the payload be copied on unlink to
     prevent introducing races in copy-down vs search-up.

 (5) The user key payload is now a structure with the data following it. It
     includes an RCU head like the keyring payload and for the same reason. It
     also contains a data length because the data length in the key may be
     changed on another CPU whilst an RCU protected read is in progress on the
     payload. This would then see the supposed RCU payload and the on-key data
     length getting out of sync.

     I'm tempted to drop the key's datalen entirely, except that it's used in
     conjunction with quota management and so is a little tricky to get rid
     of.

 (6) Update the keys documentation.

Signed-Off-By: David Howells <dhowells@redhat.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
  • Loading branch information
@dhowells
dhowells authored and Linus Torvalds committed Jun 24, 2005
1 parent 7286aa9 commit 76d8aea
Show file tree
Hide file tree
Showing 10 changed files with 480 additions and 348 deletions.
285 changes: 172 additions & 113 deletions Documentation/keys.txt
View file

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions include/linux/key-ui.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ extern spinlock_t key_serial_lock;
* subscribed
*/
struct keyring_list {
unsigned maxkeys; /* max keys this list can hold */
unsigned nkeys; /* number of keys currently held */
struct rcu_head rcu; /* RCU deletion hook */
unsigned short maxkeys; /* max keys this list can hold */
unsigned short nkeys; /* number of keys currently held */
unsigned short delkey; /* key to be unlinked by RCU */
struct key *keys[0];
};

Expand Down
25 changes: 15 additions & 10 deletions include/linux/key.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include <linux/types.h>
#include <linux/list.h>
#include <linux/rbtree.h>
#include <linux/spinlock.h>
#include <linux/rcupdate.h>
#include <asm/atomic.h>

#ifdef __KERNEL__
Expand Down Expand Up @@ -78,29 +78,32 @@ struct key {
key_serial_t serial; /* key serial number */
struct rb_node serial_node;
struct key_type *type; /* type of key */
rwlock_t lock; /* examination vs change lock */
struct rw_semaphore sem; /* change vs change sem */
struct key_user *user; /* owner of this key */
time_t expiry; /* time at which key expires (or 0) */
uid_t uid;
gid_t gid;
key_perm_t perm; /* access permissions */
unsigned short quotalen; /* length added to quota */
unsigned short datalen; /* payload data length */
unsigned short flags; /* status flags (change with lock writelocked) */
#define KEY_FLAG_INSTANTIATED 0x00000001 /* set if key has been instantiated */
#define KEY_FLAG_DEAD 0x00000002 /* set if key type has been deleted */
#define KEY_FLAG_REVOKED 0x00000004 /* set if key had been revoked */
#define KEY_FLAG_IN_QUOTA 0x00000008 /* set if key consumes quota */
#define KEY_FLAG_USER_CONSTRUCT 0x00000010 /* set if key is being constructed in userspace */
#define KEY_FLAG_NEGATIVE 0x00000020 /* set if key is negative */
unsigned short datalen; /* payload data length
* - may not match RCU dereferenced payload
* - payload should contain own length
*/

#ifdef KEY_DEBUGGING
unsigned magic;
#define KEY_DEBUG_MAGIC 0x18273645u
#define KEY_DEBUG_MAGIC_X 0xf8e9dacbu
#endif

unsigned long flags; /* status flags (change with bitops) */
#define KEY_FLAG_INSTANTIATED 0 /* set if key has been instantiated */
#define KEY_FLAG_DEAD 1 /* set if key type has been deleted */
#define KEY_FLAG_REVOKED 2 /* set if key had been revoked */
#define KEY_FLAG_IN_QUOTA 3 /* set if key consumes quota */
#define KEY_FLAG_USER_CONSTRUCT 4 /* set if key is being constructed in userspace */
#define KEY_FLAG_NEGATIVE 5 /* set if key is negative */

/* the description string
* - this is used to match a key against search criteria
* - this should be a printable string
Expand Down Expand Up @@ -250,6 +253,8 @@ extern int keyring_add_key(struct key *keyring,

extern struct key *key_lookup(key_serial_t id);

extern void keyring_replace_payload(struct key *key, void *replacement);

#define key_serial(key) ((key) ? (key)->serial : 0)

/*
Expand Down
94 changes: 42 additions & 52 deletions security/keys/key.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ struct key *key_alloc(struct key_type *type, const char *desc,
}

atomic_set(&key->usage, 1);
rwlock_init(&key->lock);
init_rwsem(&key->sem);
key->type = type;
key->user = user;
Expand All @@ -308,7 +307,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
key->payload.data = NULL;

if (!not_in_quota)
key->flags |= KEY_FLAG_IN_QUOTA;
key->flags |= 1 << KEY_FLAG_IN_QUOTA;

memset(&key->type_data, 0, sizeof(key->type_data));

Expand Down Expand Up @@ -359,7 +358,7 @@ int key_payload_reserve(struct key *key, size_t datalen)
key_check(key);

/* contemplate the quota adjustment */
if (delta != 0 && key->flags & KEY_FLAG_IN_QUOTA) {
if (delta != 0 && test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
spin_lock(&key->user->lock);

if (delta > 0 &&
Expand Down Expand Up @@ -405,23 +404,17 @@ static int __key_instantiate_and_link(struct key *key,
down_write(&key_construction_sem);

/* can't instantiate twice */
if (!(key->flags & KEY_FLAG_INSTANTIATED)) {
if (!test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
/* instantiate the key */
ret = key->type->instantiate(key, data, datalen);

if (ret == 0) {
/* mark the key as being instantiated */
write_lock(&key->lock);

atomic_inc(&key->user->nikeys);
key->flags |= KEY_FLAG_INSTANTIATED;
set_bit(KEY_FLAG_INSTANTIATED, &key->flags);

if (key->flags & KEY_FLAG_USER_CONSTRUCT) {
key->flags &= ~KEY_FLAG_USER_CONSTRUCT;
if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags))
awaken = 1;
}

write_unlock(&key->lock);

/* and link it into the destination keyring */
if (keyring)
Expand Down Expand Up @@ -486,21 +479,17 @@ int key_negate_and_link(struct key *key,
down_write(&key_construction_sem);

/* can't instantiate twice */
if (!(key->flags & KEY_FLAG_INSTANTIATED)) {
if (!test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
/* mark the key as being negatively instantiated */
write_lock(&key->lock);

atomic_inc(&key->user->nikeys);
key->flags |= KEY_FLAG_INSTANTIATED | KEY_FLAG_NEGATIVE;
set_bit(KEY_FLAG_NEGATIVE, &key->flags);
set_bit(KEY_FLAG_INSTANTIATED, &key->flags);
now = current_kernel_time();
key->expiry = now.tv_sec + timeout;

if (key->flags & KEY_FLAG_USER_CONSTRUCT) {
key->flags &= ~KEY_FLAG_USER_CONSTRUCT;
if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags))
awaken = 1;
}

write_unlock(&key->lock);
ret = 0;

/* and link it into the destination keyring */
Expand Down Expand Up @@ -553,16 +542,18 @@ static void key_cleanup(void *data)
rb_erase(&key->serial_node, &key_serial_tree);
spin_unlock(&key_serial_lock);

key_check(key);

/* deal with the user's key tracking and quota */
if (key->flags & KEY_FLAG_IN_QUOTA) {
if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
spin_lock(&key->user->lock);
key->user->qnkeys--;
key->user->qnbytes -= key->quotalen;
spin_unlock(&key->user->lock);
}

atomic_dec(&key->user->nkeys);
if (key->flags & KEY_FLAG_INSTANTIATED)
if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
atomic_dec(&key->user->nikeys);

key_user_put(key->user);
Expand Down Expand Up @@ -631,9 +622,9 @@ struct key *key_lookup(key_serial_t id)
goto error;

found:
/* pretent doesn't exist if it's dead */
/* pretend it doesn't exist if it's dead */
if (atomic_read(&key->usage) == 0 ||
(key->flags & KEY_FLAG_DEAD) ||
test_bit(KEY_FLAG_DEAD, &key->flags) ||
key->type == &key_type_dead)
goto not_found;

Expand Down Expand Up @@ -708,12 +699,9 @@ static inline struct key *__key_update(struct key *key, const void *payload,

ret = key->type->update(key, payload, plen);

if (ret == 0) {
if (ret == 0)
/* updating a negative key instantiates it */
write_lock(&key->lock);
key->flags &= ~KEY_FLAG_NEGATIVE;
write_unlock(&key->lock);
}
clear_bit(KEY_FLAG_NEGATIVE, &key->flags);

up_write(&key->sem);

Expand Down Expand Up @@ -841,12 +829,9 @@ int key_update(struct key *key, const void *payload, size_t plen)
down_write(&key->sem);
ret = key->type->update(key, payload, plen);

if (ret == 0) {
if (ret == 0)
/* updating a negative key instantiates it */
write_lock(&key->lock);
key->flags &= ~KEY_FLAG_NEGATIVE;
write_unlock(&key->lock);
}
clear_bit(KEY_FLAG_NEGATIVE, &key->flags);

up_write(&key->sem);
}
Expand Down Expand Up @@ -892,10 +877,7 @@ struct key *key_duplicate(struct key *source, const char *desc)
goto error2;

atomic_inc(&key->user->nikeys);

write_lock(&key->lock);
key->flags |= KEY_FLAG_INSTANTIATED;
write_unlock(&key->lock);
set_bit(KEY_FLAG_INSTANTIATED, &key->flags);

error_k:
up_read(&key_types_sem);
Expand All @@ -922,9 +904,7 @@ void key_revoke(struct key *key)
/* make sure no one's trying to change or use the key when we mark
* it */
down_write(&key->sem);
write_lock(&key->lock);
key->flags |= KEY_FLAG_REVOKED;
write_unlock(&key->lock);
set_bit(KEY_FLAG_REVOKED, &key->flags);
up_write(&key->sem);

} /* end key_revoke() */
Expand Down Expand Up @@ -975,24 +955,33 @@ void unregister_key_type(struct key_type *ktype)
/* withdraw the key type */
list_del_init(&ktype->link);

/* need to withdraw all keys of this type */
/* mark all the keys of this type dead */
spin_lock(&key_serial_lock);

for (_n = rb_first(&key_serial_tree); _n; _n = rb_next(_n)) {
key = rb_entry(_n, struct key, serial_node);

if (key->type != ktype)
continue;
if (key->type == ktype)
key->type = &key_type_dead;
}

spin_unlock(&key_serial_lock);

/* make sure everyone revalidates their keys */
synchronize_kernel();

/* we should now be able to destroy the payloads of all the keys of
* this type with impunity */
spin_lock(&key_serial_lock);

write_lock(&key->lock);
key->type = &key_type_dead;
write_unlock(&key->lock);
for (_n = rb_first(&key_serial_tree); _n; _n = rb_next(_n)) {
key = rb_entry(_n, struct key, serial_node);

/* there shouldn't be anyone looking at the description or
* payload now */
if (ktype->destroy)
ktype->destroy(key);
memset(&key->payload, 0xbd, sizeof(key->payload));
if (key->type == ktype) {
if (ktype->destroy)
ktype->destroy(key);
memset(&key->payload, 0xbd, sizeof(key->payload));
}
}

spin_unlock(&key_serial_lock);
Expand Down Expand Up @@ -1037,4 +1026,5 @@ void __init key_init(void)

/* link the two root keyrings together */
key_link(&root_session_keyring, &root_user_keyring);

} /* end key_init() */
23 changes: 7 additions & 16 deletions security/keys/keyctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,6 @@ long keyctl_chown_key(key_serial_t id, uid_t uid, gid_t gid)
/* make the changes with the locks held to prevent chown/chown races */
ret = -EACCES;
down_write(&key->sem);
write_lock(&key->lock);

if (!capable(CAP_SYS_ADMIN)) {
/* only the sysadmin can chown a key to some other UID */
Expand All @@ -755,7 +754,6 @@ long keyctl_chown_key(key_serial_t id, uid_t uid, gid_t gid)
ret = 0;

no_access:
write_unlock(&key->lock);
up_write(&key->sem);
key_put(key);
error:
Expand Down Expand Up @@ -784,26 +782,19 @@ long keyctl_setperm_key(key_serial_t id, key_perm_t perm)
goto error;
}

/* make the changes with the locks held to prevent chown/chmod
* races */
/* make the changes with the locks held to prevent chown/chmod races */
ret = -EACCES;
down_write(&key->sem);
write_lock(&key->lock);

/* if we're not the sysadmin, we can only chmod a key that we
* own */
if (!capable(CAP_SYS_ADMIN) && key->uid != current->fsuid)
goto no_access;

/* changing the permissions mask */
key->perm = perm;
ret = 0;
/* if we're not the sysadmin, we can only change a key that we own */
if (capable(CAP_SYS_ADMIN) || key->uid == current->fsuid) {
key->perm = perm;
ret = 0;
}

no_access:
write_unlock(&key->lock);
up_write(&key->sem);
key_put(key);
error:
error:
return ret;

} /* end keyctl_setperm_key() */
Expand Down
Loading

0 comments on commit 76d8aea

Please sign in to comment.