KEYS: Fix keyring content gc scanner

Key pointers stored in the keyring are marked in bit 1 to indicate if they point to a keyring. We need to strip off this bit before using the pointer when iterating over the keyring for the purpose of looking for links to garbage collect. This means that expirable keyrings aren't correctly expiring because the checker is seeing their key pointer with 2 added to it. Since the fix for this involves knowing about the internals of the keyring, key_gc_keyring() is moved to keyring.c and merged into keyring_gc(). This can be tested by: echo 2 >/proc/sys/kernel/keys/gc_delay keyctl timeout `keyctl add keyring qwerty "" @s` 2 cat /proc/keys sleep 5; cat /proc/keys which should see a keyring called "qwerty" appear in the session keyring and then disappear after it expires, and: echo 2 >/proc/sys/kernel/keys/gc_delay a=`keyctl get_persistent @s` b=`keyctl add keyring 0 "" $a` keyctl add user a a $b keyctl timeout $b 2 cat /proc/keys sleep 5; cat /proc/keys which should see a keyring called "0" with a key called "a" in it appear in the user's persistent keyring (which will be attached to the session keyring) and then both the "0" keyring and the "a" key should disappear when the "0" keyring expires.

David Howells committed 13 years ago

Commit 62fe3181 parent 97826c82 files · +36 −51Browse files

Unified · Split

2 files

security/keys/gc.c +1 −41
security/keys/keyring.c +35 −10

security/keys/gc.c+1−41

@@ -130,46 +130,6 @@ void key_gc_keytype(struct key_type *ktype) · 129 unmodified lines
130	130	kleave("");
131	131	}
132	132
133		−static int key_gc_keyring_func(const void object, void iterator_data)
134		−{
135		− const struct key *key = object;
136		− time_t *limit = iterator_data;
137		− return key_is_dead(key, *limit);
138		−}
139		−
140		−/*
141		− * Garbage collect pointers from a keyring.
142		− *
143		− * Not called with any locks held. The keyring's key struct will not be
144		− * deallocated under us as only our caller may deallocate it.
145		− */
146		−static void key_gc_keyring(struct key *keyring, time_t limit)
147		−{
148		− int result;
149		−
150		− kenter("%x{%s}", keyring->serial, keyring->description ?: "");
151		−
152		− if (keyring->flags & ((1 << KEY_FLAG_INVALIDATED) \|
153		− (1 << KEY_FLAG_REVOKED)))
154		− goto dont_gc;
155		−
156		− /* scan the keyring looking for dead keys */
157		− rcu_read_lock();
158		− result = assoc_array_iterate(&keyring->keys,
159		− key_gc_keyring_func, &limit);
160		− rcu_read_unlock();
161		− if (result == true)
162		− goto do_gc;
163		−
164		−dont_gc:
165		− kleave(" [no gc]");
166		− return;
167		−
168		−do_gc:
169		− keyring_gc(keyring, limit);
170		− kleave(" [gc]");
171		−}
172		−
173	133	/*
174	134	* Garbage collect a list of unreferenced, detached keys
175	135	*/
@@ -388,7 +348,7 @@ found_unreferenced_key: · 212 unmodified lines
388	348	*/
389	349	found_keyring:
390	350	spin_unlock(&key_serial_lock);
391		− key_gc_keyring(key, limit);
	351	+ keyring_gc(key, limit);
392	352	goto maybe_resched;
393	353
394	354	/* We found a dead key that is still referenced. Reset its type and

security/keys/keyring.c+35−10

@@ -1304,7 +1304,7 @@ static void keyring_revoke(struct key *keyring) · 1303 unmodified lines
1304	1304	}
1305	1305	}
1306	1306
1307		−static bool gc_iterator(void object, void iterator_data)
	1307	+static bool keyring_gc_select_iterator(void object, void iterator_data)
1308	1308	{
1309	1309	struct key *key = keyring_ptr_to_key(object);
1310	1310	time_t *limit = iterator_data;
@@ -1315,22 +1315,47 @@ static bool gc_iterator(void object, void iterator_data) · 4 unmodified lines
1315	1315	return true;
1316	1316	}
1317	1317
	1318	+static int keyring_gc_check_iterator(const void object, void iterator_data)
	1319	+{
	1320	+ const struct key *key = keyring_ptr_to_key(object);
	1321	+ time_t *limit = iterator_data;
	1322	+
	1323	+ key_check(key);
	1324	+ return key_is_dead(key, *limit);
	1325	+}
	1326	+
1318	1327	/*
1319		− * Collect garbage from the contents of a keyring, replacing the old list with
1320		− * a new one with the pointers all shuffled down.
	1328	+ * Garbage collect pointers from a keyring.
1321	1329	*
1322		− * Dead keys are classed as oned that are flagged as being dead or are revoked,
1323		− * expired or negative keys that were revoked or expired before the specified
1324		− * limit.
	1330	+ * Not called with any locks held. The keyring's key struct will not be
	1331	+ * deallocated under us as only our caller may deallocate it.
1325	1332	*/
1326	1333	void keyring_gc(struct key *keyring, time_t limit)
1327	1334	{
1328		− kenter("{%x,%s}", key_serial(keyring), keyring->description);
	1335	+ int result;
	1336	+
	1337	+ kenter("%x{%s}", keyring->serial, keyring->description ?: "");
1329	1338
	1339	+ if (keyring->flags & ((1 << KEY_FLAG_INVALIDATED) \|
	1340	+ (1 << KEY_FLAG_REVOKED)))
	1341	+ goto dont_gc;
	1342	+
	1343	+ /* scan the keyring looking for dead keys */
	1344	+ rcu_read_lock();
	1345	+ result = assoc_array_iterate(&keyring->keys,
	1346	+ keyring_gc_check_iterator, &limit);
	1347	+ rcu_read_unlock();
	1348	+ if (result == true)
	1349	+ goto do_gc;
	1350	+
	1351	+dont_gc:
	1352	+ kleave(" [no gc]");
	1353	+ return;
	1354	+
	1355	+do_gc:
1330	1356	down_write(&keyring->sem);
1331	1357	assoc_array_gc(&keyring->keys, &keyring_assoc_array_ops,
1332		− gc_iterator, &limit);
	1358	+ keyring_gc_select_iterator, &limit);
1333	1359	up_write(&keyring->sem);
1334		−
1335		− kleave("");
	1360	+ kleave(" [gc]");
1336	1361	}