forked from mirrors/linux
		
	keys: update key quotas in key_put()
Delaying key quotas update when key's refcount reaches 0 in key_put() has
been causing some issues in fscrypt testing, specifically in fstest
generic/581.  This commit fixes this test flakiness by dealing with the
quotas immediately, and leaving all the other clean-ups to the key garbage
collector.
This is done by moving the updates to the qnkeys and qnbytes fields in
struct key_user from key_gc_unused_keys() into key_put().  Unfortunately,
this also means that we need to switch to the irq-version of the spinlock
that protects these fields and use spin_lock_{irqsave,irqrestore} in all
the code that touches these fields.
Signed-off-by: Luis Henriques <lhenriques@suse.de>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@kernel.org>
			
			
This commit is contained in:
		
							parent
							
								
									45db3ab700
								
							
						
					
					
						commit
						9578e327b2
					
				
					 3 changed files with 28 additions and 23 deletions
				
			
		|  | @ -155,14 +155,6 @@ static noinline void key_gc_unused_keys(struct list_head *keys) | |||
| 
 | ||||
| 		security_key_free(key); | ||||
| 
 | ||||
| 		/* deal with the user's key tracking and quota */ | ||||
| 		if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) { | ||||
| 			spin_lock(&key->user->lock); | ||||
| 			key->user->qnkeys--; | ||||
| 			key->user->qnbytes -= key->quotalen; | ||||
| 			spin_unlock(&key->user->lock); | ||||
| 		} | ||||
| 
 | ||||
| 		atomic_dec(&key->user->nkeys); | ||||
| 		if (state != KEY_IS_UNINSTANTIATED) | ||||
| 			atomic_dec(&key->user->nikeys); | ||||
|  |  | |||
|  | @ -230,6 +230,7 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
| 	struct key *key; | ||||
| 	size_t desclen, quotalen; | ||||
| 	int ret; | ||||
| 	unsigned long irqflags; | ||||
| 
 | ||||
| 	key = ERR_PTR(-EINVAL); | ||||
| 	if (!desc || !*desc) | ||||
|  | @ -259,7 +260,7 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
| 		unsigned maxbytes = uid_eq(uid, GLOBAL_ROOT_UID) ? | ||||
| 			key_quota_root_maxbytes : key_quota_maxbytes; | ||||
| 
 | ||||
| 		spin_lock(&user->lock); | ||||
| 		spin_lock_irqsave(&user->lock, irqflags); | ||||
| 		if (!(flags & KEY_ALLOC_QUOTA_OVERRUN)) { | ||||
| 			if (user->qnkeys + 1 > maxkeys || | ||||
| 			    user->qnbytes + quotalen > maxbytes || | ||||
|  | @ -269,7 +270,7 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
| 
 | ||||
| 		user->qnkeys++; | ||||
| 		user->qnbytes += quotalen; | ||||
| 		spin_unlock(&user->lock); | ||||
| 		spin_unlock_irqrestore(&user->lock, irqflags); | ||||
| 	} | ||||
| 
 | ||||
| 	/* allocate and initialise the key and its description */ | ||||
|  | @ -327,10 +328,10 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
| 	kfree(key->description); | ||||
| 	kmem_cache_free(key_jar, key); | ||||
| 	if (!(flags & KEY_ALLOC_NOT_IN_QUOTA)) { | ||||
| 		spin_lock(&user->lock); | ||||
| 		spin_lock_irqsave(&user->lock, irqflags); | ||||
| 		user->qnkeys--; | ||||
| 		user->qnbytes -= quotalen; | ||||
| 		spin_unlock(&user->lock); | ||||
| 		spin_unlock_irqrestore(&user->lock, irqflags); | ||||
| 	} | ||||
| 	key_user_put(user); | ||||
| 	key = ERR_PTR(ret); | ||||
|  | @ -340,10 +341,10 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
| 	kmem_cache_free(key_jar, key); | ||||
| no_memory_2: | ||||
| 	if (!(flags & KEY_ALLOC_NOT_IN_QUOTA)) { | ||||
| 		spin_lock(&user->lock); | ||||
| 		spin_lock_irqsave(&user->lock, irqflags); | ||||
| 		user->qnkeys--; | ||||
| 		user->qnbytes -= quotalen; | ||||
| 		spin_unlock(&user->lock); | ||||
| 		spin_unlock_irqrestore(&user->lock, irqflags); | ||||
| 	} | ||||
| 	key_user_put(user); | ||||
| no_memory_1: | ||||
|  | @ -351,7 +352,7 @@ struct key *key_alloc(struct key_type *type, const char *desc, | |||
| 	goto error; | ||||
| 
 | ||||
| no_quota: | ||||
| 	spin_unlock(&user->lock); | ||||
| 	spin_unlock_irqrestore(&user->lock, irqflags); | ||||
| 	key_user_put(user); | ||||
| 	key = ERR_PTR(-EDQUOT); | ||||
| 	goto error; | ||||
|  | @ -380,8 +381,9 @@ int key_payload_reserve(struct key *key, size_t datalen) | |||
| 	if (delta != 0 && test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) { | ||||
| 		unsigned maxbytes = uid_eq(key->user->uid, GLOBAL_ROOT_UID) ? | ||||
| 			key_quota_root_maxbytes : key_quota_maxbytes; | ||||
| 		unsigned long flags; | ||||
| 
 | ||||
| 		spin_lock(&key->user->lock); | ||||
| 		spin_lock_irqsave(&key->user->lock, flags); | ||||
| 
 | ||||
| 		if (delta > 0 && | ||||
| 		    (key->user->qnbytes + delta > maxbytes || | ||||
|  | @ -392,7 +394,7 @@ int key_payload_reserve(struct key *key, size_t datalen) | |||
| 			key->user->qnbytes += delta; | ||||
| 			key->quotalen += delta; | ||||
| 		} | ||||
| 		spin_unlock(&key->user->lock); | ||||
| 		spin_unlock_irqrestore(&key->user->lock, flags); | ||||
| 	} | ||||
| 
 | ||||
| 	/* change the recorded data length if that didn't generate an error */ | ||||
|  | @ -645,10 +647,20 @@ void key_put(struct key *key) | |||
| 	if (key) { | ||||
| 		key_check(key); | ||||
| 
 | ||||
| 		if (refcount_dec_and_test(&key->usage)) | ||||
| 		if (refcount_dec_and_test(&key->usage)) { | ||||
| 			unsigned long flags; | ||||
| 
 | ||||
| 			/* deal with the user's key tracking and quota */ | ||||
| 			if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) { | ||||
| 				spin_lock_irqsave(&key->user->lock, flags); | ||||
| 				key->user->qnkeys--; | ||||
| 				key->user->qnbytes -= key->quotalen; | ||||
| 				spin_unlock_irqrestore(&key->user->lock, flags); | ||||
| 			} | ||||
| 			schedule_work(&key_gc_work); | ||||
| 		} | ||||
| 	} | ||||
| } | ||||
| EXPORT_SYMBOL(key_put); | ||||
| 
 | ||||
| /*
 | ||||
|  |  | |||
|  | @ -954,6 +954,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) | |||
| 	long ret; | ||||
| 	kuid_t uid; | ||||
| 	kgid_t gid; | ||||
| 	unsigned long flags; | ||||
| 
 | ||||
| 	uid = make_kuid(current_user_ns(), user); | ||||
| 	gid = make_kgid(current_user_ns(), group); | ||||
|  | @ -1010,7 +1011,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) | |||
| 			unsigned maxbytes = uid_eq(uid, GLOBAL_ROOT_UID) ? | ||||
| 				key_quota_root_maxbytes : key_quota_maxbytes; | ||||
| 
 | ||||
| 			spin_lock(&newowner->lock); | ||||
| 			spin_lock_irqsave(&newowner->lock, flags); | ||||
| 			if (newowner->qnkeys + 1 > maxkeys || | ||||
| 			    newowner->qnbytes + key->quotalen > maxbytes || | ||||
| 			    newowner->qnbytes + key->quotalen < | ||||
|  | @ -1019,12 +1020,12 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) | |||
| 
 | ||||
| 			newowner->qnkeys++; | ||||
| 			newowner->qnbytes += key->quotalen; | ||||
| 			spin_unlock(&newowner->lock); | ||||
| 			spin_unlock_irqrestore(&newowner->lock, flags); | ||||
| 
 | ||||
| 			spin_lock(&key->user->lock); | ||||
| 			spin_lock_irqsave(&key->user->lock, flags); | ||||
| 			key->user->qnkeys--; | ||||
| 			key->user->qnbytes -= key->quotalen; | ||||
| 			spin_unlock(&key->user->lock); | ||||
| 			spin_unlock_irqrestore(&key->user->lock, flags); | ||||
| 		} | ||||
| 
 | ||||
| 		atomic_dec(&key->user->nkeys); | ||||
|  | @ -1056,7 +1057,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) | |||
| 	return ret; | ||||
| 
 | ||||
| quota_overrun: | ||||
| 	spin_unlock(&newowner->lock); | ||||
| 	spin_unlock_irqrestore(&newowner->lock, flags); | ||||
| 	zapowner = newowner; | ||||
| 	ret = -EDQUOT; | ||||
| 	goto error_put; | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Luis Henriques
						Luis Henriques