Fix keyring quota misaccounting (rhbz 1017683)

2 files changed, 109 insertions, 0 deletions

diff --git a/kernel.spec b/kernel.spec
index a181d098c..165d69528 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -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
 
diff --git a/keyring-quota.patch b/keyring-quota.patch
new file mode 100644
index 000000000..07952214f
--- /dev/null
+++ b/keyring-quota.patch
@@ -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: