Browse Source

Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security

Pull keyring/nfs fixes from James Morris:
 "From David Howells:

  The first one fixes the handling of maximum buffer size for key
  descriptions, fixing the size at 4095 + NUL char rather than whatever
  PAGE_SIZE happens to be and permits you to read back the full
  description without it getting clipped because some extra information
  got prepended.

  The second and third fix a bug in NFS idmapper handling whereby a key
  representing a mapping between an id and a name expires and causing
  EKEYEXPIRED to be seen internally in NFS (which prevents the mapping
  from happening) rather than re-looking up the mapping"

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security:
  KEYS: request_key() should reget expired keys rather than give EKEYEXPIRED
  KEYS: Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags
  KEYS: Fix the size of the key description passed to/from userspace
Linus Torvalds 11 years ago
parent
commit
23c836ce5c

+ 1 - 0
security/keys/internal.h

@@ -117,6 +117,7 @@ struct keyring_search_context {
 #define KEYRING_SEARCH_NO_UPDATE_TIME	0x0004	/* Don't update times */
 #define KEYRING_SEARCH_NO_CHECK_PERM	0x0008	/* Don't check permissions */
 #define KEYRING_SEARCH_DETECT_TOO_DEEP	0x0010	/* Give an error on excessive depth */
+#define KEYRING_SEARCH_SKIP_EXPIRED	0x0020	/* Ignore expired keys (intention to replace) */
 
 	int (*iterator)(const void *object, void *iterator_data);
 

+ 26 - 30
security/keys/keyctl.c

@@ -26,6 +26,8 @@
 #include <asm/uaccess.h>
 #include "internal.h"
 
+#define KEY_MAX_DESC_SIZE 4096
+
 static int key_get_type_from_user(char *type,
 				  const char __user *_type,
 				  unsigned len)
@@ -78,7 +80,7 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type,
 
 	description = NULL;
 	if (_description) {
-		description = strndup_user(_description, PAGE_SIZE);
+		description = strndup_user(_description, KEY_MAX_DESC_SIZE);
 		if (IS_ERR(description)) {
 			ret = PTR_ERR(description);
 			goto error;
@@ -177,7 +179,7 @@ SYSCALL_DEFINE4(request_key, const char __user *, _type,
 		goto error;
 
 	/* pull the description into kernel space */
-	description = strndup_user(_description, PAGE_SIZE);
+	description = strndup_user(_description, KEY_MAX_DESC_SIZE);
 	if (IS_ERR(description)) {
 		ret = PTR_ERR(description);
 		goto error;
@@ -287,7 +289,7 @@ long keyctl_join_session_keyring(const char __user *_name)
 	/* fetch the name from userspace */
 	name = NULL;
 	if (_name) {
-		name = strndup_user(_name, PAGE_SIZE);
+		name = strndup_user(_name, KEY_MAX_DESC_SIZE);
 		if (IS_ERR(name)) {
 			ret = PTR_ERR(name);
 			goto error;
@@ -562,8 +564,9 @@ long keyctl_describe_key(key_serial_t keyid,
 {
 	struct key *key, *instkey;
 	key_ref_t key_ref;
-	char *tmpbuf;
+	char *infobuf;
 	long ret;
+	int desclen, infolen;
 
 	key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, KEY_NEED_VIEW);
 	if (IS_ERR(key_ref)) {
@@ -586,38 +589,31 @@ long keyctl_describe_key(key_serial_t keyid,
 	}
 
 okay:
-	/* calculate how much description we're going to return */
-	ret = -ENOMEM;
-	tmpbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!tmpbuf)
-		goto error2;
-
 	key = key_ref_to_ptr(key_ref);
+	desclen = strlen(key->description);
 
-	ret = snprintf(tmpbuf, PAGE_SIZE - 1,
-		       "%s;%d;%d;%08x;%s",
-		       key->type->name,
-		       from_kuid_munged(current_user_ns(), key->uid),
-		       from_kgid_munged(current_user_ns(), key->gid),
-		       key->perm,
-		       key->description ?: "");
-
-	/* include a NUL char at the end of the data */
-	if (ret > PAGE_SIZE - 1)
-		ret = PAGE_SIZE - 1;
-	tmpbuf[ret] = 0;
-	ret++;
+	/* calculate how much information we're going to return */
+	ret = -ENOMEM;
+	infobuf = kasprintf(GFP_KERNEL,
+			    "%s;%d;%d;%08x;",
+			    key->type->name,
+			    from_kuid_munged(current_user_ns(), key->uid),
+			    from_kgid_munged(current_user_ns(), key->gid),
+			    key->perm);
+	if (!infobuf)
+		goto error2;
+	infolen = strlen(infobuf);
+	ret = infolen + desclen + 1;
 
 	/* consider returning the data */
-	if (buffer && buflen > 0) {
-		if (buflen > ret)
-			buflen = ret;
-
-		if (copy_to_user(buffer, tmpbuf, buflen) != 0)
+	if (buffer && buflen >= ret) {
+		if (copy_to_user(buffer, infobuf, infolen) != 0 ||
+		    copy_to_user(buffer + infolen, key->description,
+				 desclen + 1) != 0)
 			ret = -EFAULT;
 	}
 
-	kfree(tmpbuf);
+	kfree(infobuf);
 error2:
 	key_ref_put(key_ref);
 error:
@@ -649,7 +645,7 @@ long keyctl_keyring_search(key_serial_t ringid,
 	if (ret < 0)
 		goto error;
 
-	description = strndup_user(_description, PAGE_SIZE);
+	description = strndup_user(_description, KEY_MAX_DESC_SIZE);
 	if (IS_ERR(description)) {
 		ret = PTR_ERR(description);
 		goto error;

+ 6 - 4
security/keys/keyring.c

@@ -546,7 +546,8 @@ static int keyring_search_iterator(const void *object, void *iterator_data)
 		}
 
 		if (key->expiry && ctx->now.tv_sec >= key->expiry) {
-			ctx->result = ERR_PTR(-EKEYEXPIRED);
+			if (!(ctx->flags & KEYRING_SEARCH_SKIP_EXPIRED))
+				ctx->result = ERR_PTR(-EKEYEXPIRED);
 			kleave(" = %d [expire]", ctx->skipped_ret);
 			goto skipped;
 		}
@@ -628,6 +629,10 @@ static bool search_nested_keyrings(struct key *keyring,
 	       ctx->index_key.type->name,
 	       ctx->index_key.description);
 
+#define STATE_CHECKS (KEYRING_SEARCH_NO_STATE_CHECK | KEYRING_SEARCH_DO_STATE_CHECK)
+	BUG_ON((ctx->flags & STATE_CHECKS) == 0 ||
+	       (ctx->flags & STATE_CHECKS) == STATE_CHECKS);
+
 	if (ctx->index_key.description)
 		ctx->index_key.desc_len = strlen(ctx->index_key.description);
 
@@ -637,7 +642,6 @@ static bool search_nested_keyrings(struct key *keyring,
 	if (ctx->match_data.lookup_type == KEYRING_SEARCH_LOOKUP_ITERATE ||
 	    keyring_compare_object(keyring, &ctx->index_key)) {
 		ctx->skipped_ret = 2;
-		ctx->flags |= KEYRING_SEARCH_DO_STATE_CHECK;
 		switch (ctx->iterator(keyring_key_to_ptr(keyring), ctx)) {
 		case 1:
 			goto found;
@@ -649,8 +653,6 @@ static bool search_nested_keyrings(struct key *keyring,
 	}
 
 	ctx->skipped_ret = 0;
-	if (ctx->flags & KEYRING_SEARCH_NO_STATE_CHECK)
-		ctx->flags &= ~KEYRING_SEARCH_DO_STATE_CHECK;
 
 	/* Start processing a new keyring */
 descend_to_keyring:

+ 2 - 0
security/keys/request_key.c

@@ -516,6 +516,8 @@ struct key *request_key_and_link(struct key_type *type,
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.flags			= (KEYRING_SEARCH_DO_STATE_CHECK |
+					   KEYRING_SEARCH_SKIP_EXPIRED),
 	};
 	struct key *key;
 	key_ref_t key_ref;

+ 1 - 0
security/keys/request_key_auth.c

@@ -249,6 +249,7 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
 		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
+		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
 	};
 	struct key *authkey;
 	key_ref_t authkey_ref;