summaryrefslogtreecommitdiffstats
path: root/security
diff options
context:
space:
mode:
authorDavid Howells <dhowells@redhat.com>2014-12-01 22:52:45 +0000
committerDavid Howells <dhowells@redhat.com>2014-12-01 22:52:45 +0000
commitaa9d4437893f7e015ce5b6d6c443a9ba92c8a2e7 (patch)
tree40cb7d4a6ff0b231a10cf7e51c8a8b24a7f3cd45 /security
parent009d0431c3914de64666bec0d350e54fdd59df6a (diff)
downloadop-kernel-dev-aa9d4437893f7e015ce5b6d6c443a9ba92c8a2e7.zip
op-kernel-dev-aa9d4437893f7e015ce5b6d6c443a9ba92c8a2e7.tar.gz
KEYS: Fix the size of the key description passed to/from userspace
When a key description argument is imported into the kernel from userspace, as happens in add_key(), request_key(), KEYCTL_JOIN_SESSION_KEYRING, KEYCTL_SEARCH, the description is copied into a buffer up to PAGE_SIZE in size. PAGE_SIZE, however, is a variable quantity, depending on the arch. Fix this at 4096 instead (ie. 4095 plus a NUL termination) and define a constant (KEY_MAX_DESC_SIZE) to this end. When reading the description back with KEYCTL_DESCRIBE, a PAGE_SIZE internal buffer is allocated into which the information and description will be rendered. This means that the description will get truncated if an extremely long description it has to be crammed into the buffer with the stringified information. There is no particular need to copy the description into the buffer, so just copy it directly to userspace in a separate operation. Reported-by: Christian Kastner <debian@kvr.at> Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Christian Kastner <debian@kvr.at>
Diffstat (limited to 'security')
-rw-r--r--security/keys/keyctl.c56
1 files changed, 26 insertions, 30 deletions
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index eff88a5..4743d71 100644
--- a/security/keys/keyctl.c
+++ b/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;
OpenPOWER on IntegriCloud