Fix keyring quota misaccounting (rhbz 1017683)

This commit is contained in:
Josh Boyer 2013-10-18 14:56:50 -04:00
parent 34e7db2489
commit 8664f7c020
2 changed files with 109 additions and 0 deletions

View File

@ -648,6 +648,7 @@ Patch800: crash-driver.patch
Patch900: keys-expand-keyring.patch
Patch901: keys-krb-support.patch
Patch902: keys-x509-improv.patch
Patch903: keyring-quota.patch
# secure boot
Patch1000: secure-modules.patch
@ -1371,6 +1372,7 @@ ApplyPatch crash-driver.patch
ApplyPatch keys-expand-keyring.patch
ApplyPatch keys-krb-support.patch
ApplyPatch keys-x509-improv.patch
ApplyPatch keyring-quota.patch
# secure boot
ApplyPatch secure-modules.patch
@ -2262,6 +2264,9 @@ fi
# ||----w |
# || ||
%changelog
* Fri Oct 18 2013 Josh Boyer <jwboyer@fedoraproject.org>
- Fix keyring quota misaccounting (rhbz 1017683)
* Thu Oct 17 2013 Josh Boyer <jwboyer@fedoraproject.org> - 3.12.0-0.rc5.git3.1
- Linux v3.12-rc5-78-g056cdce

104
keyring-quota.patch Normal file
View File

@ -0,0 +1,104 @@
commit cb3bd4d9775d833501826832fd1562af19f8182d
Author: David Howells <dhowells@redhat.com>
Date: Fri Oct 18 17:30:30 2013 +0100
KEYS: Fix keyring quota misaccounting on key replacement and unlink
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>
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 8c05ebd..d80311e 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -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: