Skip to content

Commit 65d87fe

Browse files
dhowellsdhowells
committed
KEYS: Perform RCU synchronisation on keys prior to key destruction
Make the keys garbage collector invoke synchronize_rcu() prior to destroying keys with a zero usage count. This means that a key can be examined under the RCU read lock in the safe knowledge that it won't get deallocated until after the lock is released - even if its usage count becomes zero whilst we're looking at it. This is useful in keyring search vs key link. Consider a keyring containing a link to a key. That link can be replaced in-place in the keyring without requiring an RCU copy-and-replace on the keyring contents without breaking a search underway on that keyring when the displaced key is released, provided the key is actually destroyed only after the RCU read lock held by the search algorithm is released. This permits __key_link() to replace a key without having to reallocate the key payload. A key gets replaced if a new key being linked into a keyring has the same type and description. Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Jeff Layton <jlayton@redhat.com>
1 parent 1eb1bcf commit 65d87fe

File tree

2 files changed

+48
-30
lines changed

2 files changed

+48
-30
lines changed

‎include/linux/key.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ static inline unsigned long is_key_possessed(const key_ref_t key_ref)
124124
struct key {
125125
atomic_t usage; /* number of references */
126126
key_serial_t serial; /* key serial number */
127-
struct rb_node serial_node;
127+
union {
128+
struct list_head graveyard_link;
129+
struct rb_node serial_node;
130+
};
128131
struct key_type *type; /* type of key */
129132
struct rw_semaphore sem; /* change vs change sem */
130133
struct key_user *user; /* owner of this key */

‎security/keys/gc.c

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -168,38 +168,45 @@ static void key_gc_keyring(struct key *keyring, time_t limit)
168168
}
169169

170170
/*
171-
* Garbage collect an unreferenced, detached key
171+
* Garbage collect a list of unreferenced, detached keys
172172
*/
173-
static noinline void key_gc_unused_key(struct key *key)
173+
static noinline void key_gc_unused_keys(struct list_head *keys)
174174
{
175-
key_check(key);
176-
177-
security_key_free(key);
178-
179-
/* deal with the user's key tracking and quota */
180-
if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
181-
spin_lock(&key->user->lock);
182-
key->user->qnkeys--;
183-
key->user->qnbytes -= key->quotalen;
184-
spin_unlock(&key->user->lock);
185-
}
175+
while (!list_empty(keys)) {
176+
struct key *key =
177+
list_entry(keys->next, struct key, graveyard_link);
178+
list_del(&key->graveyard_link);
179+
180+
kdebug("- %u", key->serial);
181+
key_check(key);
182+
183+
security_key_free(key);
184+
185+
/* deal with the user's key tracking and quota */
186+
if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
187+
spin_lock(&key->user->lock);
188+
key->user->qnkeys--;
189+
key->user->qnbytes -= key->quotalen;
190+
spin_unlock(&key->user->lock);
191+
}
186192

187-
atomic_dec(&key->user->nkeys);
188-
if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
189-
atomic_dec(&key->user->nikeys);
193+
atomic_dec(&key->user->nkeys);
194+
if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
195+
atomic_dec(&key->user->nikeys);
190196

191-
key_user_put(key->user);
197+
key_user_put(key->user);
192198

193-
/* now throw away the key memory */
194-
if (key->type->destroy)
195-
key->type->destroy(key);
199+
/* now throw away the key memory */
200+
if (key->type->destroy)
201+
key->type->destroy(key);
196202

197-
kfree(key->description);
203+
kfree(key->description);
198204

199205
#ifdef KEY_DEBUGGING
200-
key->magic = KEY_DEBUG_MAGIC_X;
206+
key->magic = KEY_DEBUG_MAGIC_X;
201207
#endif
202-
kmem_cache_free(key_jar, key);
208+
kmem_cache_free(key_jar, key);
209+
}
203210
}
204211

205212
/*
@@ -211,6 +218,7 @@ static noinline void key_gc_unused_key(struct key *key)
211218
*/
212219
static void key_garbage_collector(struct work_struct *work)
213220
{
221+
static LIST_HEAD(graveyard);
214222
static u8 gc_state; /* Internal persistent state */
215223
#define KEY_GC_REAP_AGAIN 0x01 /* - Need another cycle */
216224
#define KEY_GC_REAPING_LINKS 0x02 /* - We need to reap links */
@@ -316,15 +324,22 @@ static void key_garbage_collector(struct work_struct *work)
316324
key_schedule_gc(new_timer);
317325
}
318326

319-
if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2)) {
320-
/* Make sure everyone revalidates their keys if we marked a
321-
* bunch as being dead and make sure all keyring ex-payloads
322-
* are destroyed.
327+
if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2) ||
328+
!list_empty(&graveyard)) {
329+
/* Make sure that all pending keyring payload destructions are
330+
* fulfilled and that people aren't now looking at dead or
331+
* dying keys that they don't have a reference upon or a link
332+
* to.
323333
*/
324-
kdebug("dead sync");
334+
kdebug("gc sync");
325335
synchronize_rcu();
326336
}
327337

338+
if (!list_empty(&graveyard)) {
339+
kdebug("gc keys");
340+
key_gc_unused_keys(&graveyard);
341+
}
342+
328343
if (unlikely(gc_state & (KEY_GC_REAPING_DEAD_1 |
329344
KEY_GC_REAPING_DEAD_2))) {
330345
if (!(gc_state & KEY_GC_FOUND_DEAD_KEY)) {
@@ -359,7 +374,7 @@ static void key_garbage_collector(struct work_struct *work)
359374
rb_erase(&key->serial_node, &key_serial_tree);
360375
spin_unlock(&key_serial_lock);
361376

362-
key_gc_unused_key(key);
377+
list_add_tail(&key->graveyard_link, &graveyard);
363378
gc_state |= KEY_GC_REAP_AGAIN;
364379
goto maybe_resched;
365380

0 commit comments

Comments
 (0)