| From 3239b6f29bdfb4b0a2ba59df995fc9e6f4df7f1f Mon Sep 17 00:00:00 2001 |
| From: Eric Biggers <ebiggers@google.com> |
| Date: Thu, 2 Nov 2017 00:47:03 +0000 |
| Subject: KEYS: return full count in keyring_read() if buffer is too small |
| |
| From: Eric Biggers <ebiggers@google.com> |
| |
| 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> |
| |
| --- |
| security/keys/keyring.c | 39 +++++++++++++++++++-------------------- |
| 1 file changed, 19 insertions(+), 20 deletions(-) |
| |
| --- a/security/keys/keyring.c |
| +++ b/security/keys/keyring.c |
| @@ -452,34 +452,33 @@ static long keyring_read(const struct ke |
| 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; |
| } |
| |
| /* |