8664f7
commit cb3bd4d9775d833501826832fd1562af19f8182d
8664f7
Author: David Howells <dhowells@redhat.com>
8664f7
Date:   Fri Oct 18 17:30:30 2013 +0100
8664f7
8664f7
    KEYS: Fix keyring quota misaccounting on key replacement and unlink
8664f7
    
8664f7
    If a key is displaced from a keyring by a matching one, then four more bytes
8664f7
    of quota are allocated to the keyring - despite the fact that the keyring does
8664f7
    not change in size.
8664f7
    
8664f7
    Further, when a key is unlinked from a keyring, the four bytes of quota
8664f7
    allocated the link isn't recovered and returned to the user's pool.
8664f7
    
8664f7
    The first can be tested by repeating:
8664f7
    
8664f7
    	keyctl add big_key a fred @s
8664f7
    	cat /proc/key-users
8664f7
    
8664f7
    (Don't put it in a shell loop otherwise the garbage collector won't have time
8664f7
    to clear the displaced keys, thus affecting the result).
8664f7
    
8664f7
    This was causing the kerberos keyring to run out of room fairly quickly.
8664f7
    
8664f7
    The second can be tested by:
8664f7
    
8664f7
    	cat /proc/key-users
8664f7
    	a=`keyctl add user a a @s`
8664f7
    	cat /proc/key-users
8664f7
    	keyctl unlink $a
8664f7
    	sleep 1 # Give RCU a chance to delete the key
8664f7
    	cat /proc/key-users
8664f7
    
8664f7
    assuming no system activity that otherwise adds/removes keys, the amount of
8664f7
    key data allocated should go up (say 40/20000 -> 47/20000) and then return to
8664f7
    the original value at the end.
8664f7
    
8664f7
    Reported-by: Stephen Gallagher <sgallagh@redhat.com>
8664f7
    Signed-off-by: David Howells <dhowells@redhat.com>
8664f7
8664f7
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
8664f7
index 8c05ebd..d80311e 100644
8664f7
--- a/security/keys/keyring.c
8664f7
+++ b/security/keys/keyring.c
8664f7
@@ -1063,12 +1063,6 @@ int __key_link_begin(struct key *keyring,
8664f7
 	if (index_key->type == &key_type_keyring)
8664f7
 		down_write(&keyring_serialise_link_sem);
8664f7
 
8664f7
-	/* check that we aren't going to overrun the user's quota */
8664f7
-	ret = key_payload_reserve(keyring,
8664f7
-				  keyring->datalen + KEYQUOTA_LINK_BYTES);
8664f7
-	if (ret < 0)
8664f7
-		goto error_sem;
8664f7
-
8664f7
 	/* Create an edit script that will insert/replace the key in the
8664f7
 	 * keyring tree.
8664f7
 	 */
8664f7
@@ -1078,17 +1072,25 @@ int __key_link_begin(struct key *keyring,
8664f7
 				  NULL);
8664f7
 	if (IS_ERR(edit)) {
8664f7
 		ret = PTR_ERR(edit);
8664f7
-		goto error_quota;
8664f7
+		goto error_sem;
8664f7
+	}
8664f7
+
8664f7
+	/* If we're not replacing a link in-place then we're going to need some
8664f7
+	 * extra quota.
8664f7
+	 */
8664f7
+	if (!edit->dead_leaf) {
8664f7
+		ret = key_payload_reserve(keyring,
8664f7
+					  keyring->datalen + KEYQUOTA_LINK_BYTES);
8664f7
+		if (ret < 0)
8664f7
+			goto error_cancel;
8664f7
 	}
8664f7
 
8664f7
 	*_edit = edit;
8664f7
 	kleave(" = 0");
8664f7
 	return 0;
8664f7
 
8664f7
-error_quota:
8664f7
-	/* undo the quota changes */
8664f7
-	key_payload_reserve(keyring,
8664f7
-			    keyring->datalen - KEYQUOTA_LINK_BYTES);
8664f7
+error_cancel:
8664f7
+	assoc_array_cancel_edit(edit);
8664f7
 error_sem:
8664f7
 	if (index_key->type == &key_type_keyring)
8664f7
 		up_write(&keyring_serialise_link_sem);
8664f7
@@ -1146,7 +1148,7 @@ void __key_link_end(struct key *keyring,
8664f7
 	if (index_key->type == &key_type_keyring)
8664f7
 		up_write(&keyring_serialise_link_sem);
8664f7
 
8664f7
-	if (edit) {
8664f7
+	if (edit && !edit->dead_leaf) {
8664f7
 		key_payload_reserve(keyring,
8664f7
 				    keyring->datalen - KEYQUOTA_LINK_BYTES);
8664f7
 		assoc_array_cancel_edit(edit);
8664f7
@@ -1243,6 +1245,7 @@ int key_unlink(struct key *keyring, struct key *key)
8664f7
 		goto error;
8664f7
 
8664f7
 	assoc_array_apply_edit(edit);
8664f7
+	key_payload_reserve(keyring, keyring->datalen - KEYQUOTA_LINK_BYTES);
8664f7
 	ret = 0;
8664f7
 
8664f7
 error: