KEYS: Fix __key_link_end() quota fixup on error

Fix __key_link_end()'s attempt to fix up the quota if an error occurs. There are two erroneous cases: Firstly, we always decrease the quota if the preallocated replacement keyring needs cleaning up, irrespective of whether or not we should (we may have replaced a pointer rather than adding another pointer). Secondly, we never clean up the quota if we added a pointer without the keyring storage being extended (we allocate multiple pointers at a time, even if we're not going to use them all immediately). We handle this by setting the bottom bit of the preallocation pointer in __key_link_begin() to indicate that the quota needs fixing up, which is then passed to __key_link() (which clears the whole thing) and __key_link_end(). Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

KEYS: Fix __key_link_end() quota fixup on error
Fix __key_link_end()'s attempt to fix up the quota if an error occurs. There are two erroneous cases: Firstly, we always decrease the quota if the preallocated replacement keyring needs cleaning up, irrespective of whether or not we should (we may have replaced a pointer rather than adding another pointer). Secondly, we never clean up the quota if we added a pointer without the keyring storage being extended (we allocate multiple pointers at a time, even if we're not going to use them all immediately). We handle this by setting the bottom bit of the preallocation pointer in __key_link_begin() to indicate that the quota needs fixing up, which is then passed to __key_link() (which clears the whole thing) and __key_link_end(). Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
David Howells · Linus Torvalds
1 parent f5c66d70ac
Showing 4 changed files with 27 additions and 20 deletions Side-by-side Diff
security/keys/internal.h
security/keys/key.c
security/keys/keyring.c
security/keys/request_key.c
@@ -87,13 +87,13 @@
 extern int __key_link_begin(struct key *keyring,
 			    const struct key_type *type,
 			    const char *description,
-			    struct keyring_list **_prealloc);
+			    unsigned long *_prealloc);
 extern int __key_link_check_live_key(struct key *keyring, struct key *key);
 extern void __key_link(struct key *keyring, struct key *key,
-		       struct keyring_list **_prealloc);
+		       unsigned long *_prealloc);
 extern void __key_link_end(struct key *keyring,
 			   struct key_type *type,
-			   struct keyring_list *prealloc);
+			   unsigned long prealloc);
  
 extern key_ref_t __keyring_search_one(key_ref_t keyring_ref,
 				      const struct key_type *type,
@@ -415,7 +415,7 @@
 				      size_t datalen,
 				      struct key *keyring,
 				      struct key *authkey,
-				      struct keyring_list **_prealloc)
+				      unsigned long *_prealloc)
 {
 	int ret, awaken;
  
@@ -481,7 +481,7 @@
 			     struct key *keyring,
 			     struct key *authkey)
 {
-	struct keyring_list *prealloc;
+	unsigned long prealloc;
 	int ret;
  
 	if (keyring) {
@@ -526,7 +526,7 @@
 			struct key *keyring,
 			struct key *authkey)
 {
-	struct keyring_list *prealloc;
+	unsigned long prealloc;
 	struct timespec now;
 	int ret, awaken, link_ret = 0;
  
@@ -814,7 +814,7 @@
 			       key_perm_t perm,
 			       unsigned long flags)
 {
-	struct keyring_list *prealloc;
+	unsigned long prealloc;
 	const struct cred *cred = current_cred();
 	struct key_type *ktype;
 	struct key *keyring, *key = NULL;
@@ -25,6 +25,8 @@
 		(keyring)->payload.subscriptions,			\
 		rwsem_is_locked((struct rw_semaphore *)&(keyring)->sem)))
  
+#define KEY_LINK_FIXQUOTA 1UL
+
 /*
  * When plumbing the depths of the key tree, this sets a hard limit
  * set on how deep we're willing to go.
  
@@ -699,11 +701,11 @@
  * Preallocate memory so that a key can be linked into to a keyring.
  */
 int __key_link_begin(struct key *keyring, const struct key_type *type,
-		     const char *description,
-		     struct keyring_list **_prealloc)
+		     const char *description, unsigned long *_prealloc)
 	__acquires(&keyring->sem)
 {
 	struct keyring_list *klist, *nklist;
+	unsigned long prealloc;
 	unsigned max;
 	size_t size;
 	int loop, ret;
@@ -746,6 +748,7 @@
  
 				/* note replacement slot */
 				klist->delkey = nklist->delkey = loop;
+				prealloc = (unsigned long)nklist;
 				goto done;
 			}
 		}
@@ -760,6 +763,7 @@
 	if (klist && klist->nkeys < klist->maxkeys) {
 		/* there's sufficient slack space to append directly */
 		nklist = NULL;
+		prealloc = KEY_LINK_FIXQUOTA;
 	} else {
 		/* grow the key list */
 		max = 4;
  
@@ -794,8 +798,9 @@
 		nklist->keys[nklist->delkey] = NULL;
 	}
  
+	prealloc = (unsigned long)nklist | KEY_LINK_FIXQUOTA;
 done:
-	*_prealloc = nklist;
+	*_prealloc = prealloc;
 	kleave(" = 0");
 	return 0;
  
  
@@ -836,12 +841,12 @@
  * combination.
  */
 void __key_link(struct key *keyring, struct key *key,
-		struct keyring_list **_prealloc)
+		unsigned long *_prealloc)
 {
 	struct keyring_list *klist, *nklist;
  
-	nklist = *_prealloc;
-	*_prealloc = NULL;
+	nklist = (struct keyring_list *)(*_prealloc & ~KEY_LINK_FIXQUOTA);
+	*_prealloc = 0;
  
 	kenter("%d,%d,%p", keyring->serial, key->serial, nklist);
  
  
  
@@ -881,20 +886,22 @@
  * Must be called with __key_link_begin() having being called.
  */
 void __key_link_end(struct key *keyring, struct key_type *type,
-		    struct keyring_list *prealloc)
+		    unsigned long prealloc)
 	__releases(&keyring->sem)
 {
 	BUG_ON(type == NULL);
 	BUG_ON(type->name == NULL);
-	kenter("%d,%s,%p", keyring->serial, type->name, prealloc);
+	kenter("%d,%s,%lx", keyring->serial, type->name, prealloc);
  
 	if (type == &key_type_keyring)
 		up_write(&keyring_serialise_link_sem);
  
 	if (prealloc) {
-		kfree(prealloc);
-		key_payload_reserve(keyring,
-				    keyring->datalen - KEYQUOTA_LINK_BYTES);
+		if (prealloc & KEY_LINK_FIXQUOTA)
+			key_payload_reserve(keyring,
+					    keyring->datalen -
+					    KEYQUOTA_LINK_BYTES);
+		kfree((struct keyring_list *)(prealloc & ~KEY_LINK_FIXQUOTA));
 	}
 	up_write(&keyring->sem);
 }
@@ -921,7 +928,7 @@
  */
 int key_link(struct key *keyring, struct key *key)
 {
-	struct keyring_list *prealloc;
+	unsigned long prealloc;
 	int ret;
  
 	key_check(keyring);
@@ -352,8 +352,8 @@
 			       struct key_user *user,
 			       struct key **_key)
 {
-	struct keyring_list *prealloc;
 	const struct cred *cred = current_cred();
+	unsigned long prealloc;
 	struct key *key;
 	key_ref_t key_ref;
 	int ret;
...	...	@@ -87,13 +87,13 @@
87	87	extern int __key_link_begin(struct key *keyring,
88	88	const struct key_type *type,
89	89	const char *description,
90		- struct keyring_list **_prealloc);
	90	+ unsigned long *_prealloc);
91	91	extern int __key_link_check_live_key(struct key keyring, struct key key);
92	92	extern void __key_link(struct key keyring, struct key key,
93		- struct keyring_list **_prealloc);
	93	+ unsigned long *_prealloc);
94	94	extern void __key_link_end(struct key *keyring,
95	95	struct key_type *type,
96		- struct keyring_list *prealloc);
	96	+ unsigned long prealloc);
97	97
98	98	extern key_ref_t __keyring_search_one(key_ref_t keyring_ref,
99	99	const struct key_type *type,
...	...	@@ -415,7 +415,7 @@
415	415	size_t datalen,
416	416	struct key *keyring,
417	417	struct key *authkey,
418		- struct keyring_list **_prealloc)
	418	+ unsigned long *_prealloc)
419	419	{
420	420	int ret, awaken;
421	421
...	...	@@ -481,7 +481,7 @@
481	481	struct key *keyring,
482	482	struct key *authkey)
483	483	{
484		- struct keyring_list *prealloc;
	484	+ unsigned long prealloc;
485	485	int ret;
486	486
487	487	if (keyring) {
...	...	@@ -526,7 +526,7 @@
526	526	struct key *keyring,
527	527	struct key *authkey)
528	528	{
529		- struct keyring_list *prealloc;
	529	+ unsigned long prealloc;
530	530	struct timespec now;
531	531	int ret, awaken, link_ret = 0;
532	532
...	...	@@ -814,7 +814,7 @@
814	814	key_perm_t perm,
815	815	unsigned long flags)
816	816	{
817		- struct keyring_list *prealloc;
	817	+ unsigned long prealloc;
818	818	const struct cred *cred = current_cred();
819	819	struct key_type *ktype;
820	820	struct key keyring, key = NULL;
...	...	@@ -25,6 +25,8 @@
25	25	(keyring)->payload.subscriptions, \
26	26	rwsem_is_locked((struct rw_semaphore *)&(keyring)->sem)))
27	27
	28	+#define KEY_LINK_FIXQUOTA 1UL
	29	+
28	30	/*
29	31	* When plumbing the depths of the key tree, this sets a hard limit
30	32	* set on how deep we're willing to go.
31	33
...	...	@@ -699,11 +701,11 @@
699	701	* Preallocate memory so that a key can be linked into to a keyring.
700	702	*/
701	703	int __key_link_begin(struct key keyring, const struct key_type type,
702		- const char *description,
703		- struct keyring_list **_prealloc)
	704	+ const char description, unsigned long _prealloc)
704	705	__acquires(&keyring->sem)
705	706	{
706	707	struct keyring_list klist, nklist;
	708	+ unsigned long prealloc;
707	709	unsigned max;
708	710	size_t size;
709	711	int loop, ret;
...	...	@@ -746,6 +748,7 @@
746	748
747	749	/* note replacement slot */
748	750	klist->delkey = nklist->delkey = loop;
	751	+ prealloc = (unsigned long)nklist;
749	752	goto done;
750	753	}
751	754	}
...	...	@@ -760,6 +763,7 @@
760	763	if (klist && klist->nkeys < klist->maxkeys) {
761	764	/* there's sufficient slack space to append directly */
762	765	nklist = NULL;
	766	+ prealloc = KEY_LINK_FIXQUOTA;
763	767	} else {
764	768	/* grow the key list */
765	769	max = 4;
766	770
...	...	@@ -794,8 +798,9 @@
794	798	nklist->keys[nklist->delkey] = NULL;
795	799	}
796	800
	801	+ prealloc = (unsigned long)nklist \| KEY_LINK_FIXQUOTA;
797	802	done:
798		- *_prealloc = nklist;
	803	+ *_prealloc = prealloc;
799	804	kleave(" = 0");
800	805	return 0;
801	806
802	807
...	...	@@ -836,12 +841,12 @@
836	841	* combination.
837	842	*/
838	843	void __key_link(struct key keyring, struct key key,
839		- struct keyring_list **_prealloc)
	844	+ unsigned long *_prealloc)
840	845	{
841	846	struct keyring_list klist, nklist;
842	847
843		- nklist = *_prealloc;
844		- *_prealloc = NULL;
	848	+ nklist = (struct keyring_list )(_prealloc & ~KEY_LINK_FIXQUOTA);
	849	+ *_prealloc = 0;
845	850
846	851	kenter("%d,%d,%p", keyring->serial, key->serial, nklist);
847	852
848	853
849	854
...	...	@@ -881,20 +886,22 @@
881	886	* Must be called with __key_link_begin() having being called.
882	887	*/
883	888	void __key_link_end(struct key keyring, struct key_type type,
884		- struct keyring_list *prealloc)
	889	+ unsigned long prealloc)
885	890	__releases(&keyring->sem)
886	891	{
887	892	BUG_ON(type == NULL);
888	893	BUG_ON(type->name == NULL);
889		- kenter("%d,%s,%p", keyring->serial, type->name, prealloc);
	894	+ kenter("%d,%s,%lx", keyring->serial, type->name, prealloc);
890	895
891	896	if (type == &key_type_keyring)
892	897	up_write(&keyring_serialise_link_sem);
893	898
894	899	if (prealloc) {
895		- kfree(prealloc);
896		- key_payload_reserve(keyring,
897		- keyring->datalen - KEYQUOTA_LINK_BYTES);
	900	+ if (prealloc & KEY_LINK_FIXQUOTA)
	901	+ key_payload_reserve(keyring,
	902	+ keyring->datalen -
	903	+ KEYQUOTA_LINK_BYTES);
	904	+ kfree((struct keyring_list *)(prealloc & ~KEY_LINK_FIXQUOTA));
898	905	}
899	906	up_write(&keyring->sem);
900	907	}
...	...	@@ -921,7 +928,7 @@
921	928	*/
922	929	int key_link(struct key keyring, struct key key)
923	930	{
924		- struct keyring_list *prealloc;
	931	+ unsigned long prealloc;
925	932	int ret;
926	933
927	934	key_check(keyring);
...	...	@@ -352,8 +352,8 @@
352	352	struct key_user *user,
353	353	struct key **_key)
354	354	{
355		- struct keyring_list *prealloc;
356	355	const struct cred *cred = current_cred();
	356	+ unsigned long prealloc;
357	357	struct key *key;
358	358	key_ref_t key_ref;
359	359	int ret;