105 lines
		
	
	
		
			3.2 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			105 lines
		
	
	
		
			3.2 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
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:
 |