KEYS: Fix keyring quota misaccounting on key replacement and unlink (034faeb9) · Commits · 戴 / test

If a key is displaced from a keyring by a matching one, then four more bytes of quota are allocated to the keyring - despite the fact that the keyring does not change in size. Further, when a key is unlinked from a keyring, the four bytes of quota allocated the link isn't recovered and returned to the user's pool. The first can be tested by repeating: keyctl add big_key a fred @s cat /proc/key-users (Don't put it in a shell loop otherwise the garbage collector won't have time to clear the displaced keys, thus affecting the result). This was causing the kerberos keyring to run out of room fairly quickly. The second can be tested by: cat /proc/key-users a=`keyctl add user a a @s` cat /proc/key-users keyctl unlink $a sleep 1 # Give RCU a chance to delete the key cat /proc/key-users assuming no system activity that otherwise adds/removes keys, the amount of key data allocated should go up (say 40/20000 -> 47/20000) and then return to the original value at the end. Reported-by:

Stephen Gallagher <sgallagh@redhat.com> Signed-off-by:

David Howells <dhowells@redhat.com>

security/keys/keyring.c

+15 −12

Original line number	Diff line number	Diff line
		@@ -1063,12 +1063,6 @@ int __key_link_begin(struct key *keyring,
		if (index_key->type == &key_type_keyring)
		down_write(&keyring_serialise_link_sem);

		/* check that we aren't going to overrun the user's quota */
		ret = key_payload_reserve(keyring,
		keyring->datalen + KEYQUOTA_LINK_BYTES);
		if (ret < 0)
		goto error_sem;

		/* Create an edit script that will insert/replace the key in the
		* keyring tree.
		*/
		@@ -1078,17 +1072,25 @@ int __key_link_begin(struct key *keyring,
		NULL);
		if (IS_ERR(edit)) {
		ret = PTR_ERR(edit);
		goto error_quota;
		goto error_sem;
		}

		/* If we're not replacing a link in-place then we're going to need some
		* extra quota.
		*/
		if (!edit->dead_leaf) {
		ret = key_payload_reserve(keyring,
		keyring->datalen + KEYQUOTA_LINK_BYTES);
		if (ret < 0)
		goto error_cancel;
		}

		*_edit = edit;
		kleave(" = 0");
		return 0;

		error_quota:
		/* undo the quota changes */
		key_payload_reserve(keyring,
		keyring->datalen - KEYQUOTA_LINK_BYTES);
		error_cancel:
		assoc_array_cancel_edit(edit);
		error_sem:
		if (index_key->type == &key_type_keyring)
		up_write(&keyring_serialise_link_sem);
		@@ -1146,7 +1148,7 @@ void __key_link_end(struct key *keyring,
		if (index_key->type == &key_type_keyring)
		up_write(&keyring_serialise_link_sem);

		if (edit) {
		if (edit && !edit->dead_leaf) {
		key_payload_reserve(keyring,
		keyring->datalen - KEYQUOTA_LINK_BYTES);
		assoc_array_cancel_edit(edit);
		@@ -1243,6 +1245,7 @@ int key_unlink(struct key keyring, struct key key)
		goto error;

		assoc_array_apply_edit(edit);
		key_payload_reserve(keyring, keyring->datalen - KEYQUOTA_LINK_BYTES);
		ret = 0;

		error:

Admin message