summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Biggers <ebiggers@google.com>2017-11-02 03:47:03 +0300
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2017-11-08 12:17:16 +0300
commit4988317d1fc062e2b75195a25c24352b2f6e59ee (patch)
tree43f2faadf499aa7f394bfc99c7ea47ef08ebfe12
parent7d64e01cf2b626a76c4c465fc64f1107dded8441 (diff)
downloadlinux-4988317d1fc062e2b75195a25c24352b2f6e59ee.tar.xz
KEYS: return full count in keyring_read() if buffer is too small
commit 3239b6f29bdfb4b0a2ba59df995fc9e6f4df7f1f upstream. Commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()") made keyring_read() stop corrupting userspace memory when the user-supplied buffer is too small. However it also made the return value in that case be the short buffer size rather than the size required, yet keyctl_read() is actually documented to return the size required. Therefore, switch it over to the documented behavior. Note that for now we continue to have it fill the short buffer, since it did that before (pre-v3.13) and dump_key_tree_aux() in keyutils arguably relies on it. Fixes: e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()") Reported-by: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: James Morris <james.l.morris@oracle.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--security/keys/keyring.c39
1 files changed, 19 insertions, 20 deletions
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 06173b091a74..c04032302a25 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -459,34 +459,33 @@ static long keyring_read(const struct key *keyring,
char __user *buffer, size_t buflen)
{
struct keyring_read_iterator_context ctx;
- unsigned long nr_keys;
- int ret;
+ long ret;
kenter("{%d},,%zu", key_serial(keyring), buflen);
if (buflen & (sizeof(key_serial_t) - 1))
return -EINVAL;
- nr_keys = keyring->keys.nr_leaves_on_tree;
- if (nr_keys == 0)
- return 0;
-
- /* Calculate how much data we could return */
- if (!buffer || !buflen)
- return nr_keys * sizeof(key_serial_t);
-
- /* Copy the IDs of the subscribed keys into the buffer */
- ctx.buffer = (key_serial_t __user *)buffer;
- ctx.buflen = buflen;
- ctx.count = 0;
- ret = assoc_array_iterate(&keyring->keys, keyring_read_iterator, &ctx);
- if (ret < 0) {
- kleave(" = %d [iterate]", ret);
- return ret;
+ /* Copy as many key IDs as fit into the buffer */
+ if (buffer && buflen) {
+ ctx.buffer = (key_serial_t __user *)buffer;
+ ctx.buflen = buflen;
+ ctx.count = 0;
+ ret = assoc_array_iterate(&keyring->keys,
+ keyring_read_iterator, &ctx);
+ if (ret < 0) {
+ kleave(" = %ld [iterate]", ret);
+ return ret;
+ }
}
- kleave(" = %zu [ok]", ctx.count);
- return ctx.count;
+ /* Return the size of the buffer needed */
+ ret = keyring->keys.nr_leaves_on_tree * sizeof(key_serial_t);
+ if (ret <= buflen)
+ kleave("= %ld [ok]", ret);
+ else
+ kleave("= %ld [buffer too small]", ret);
+ return ret;
}
/*