2013-04-14 16:12:56

by Milan Broz

[permalink] [raw]
Subject: [PATCH] algif_skcipher: Avoid crash if buffer is not multiple of cipher block size

When user requests encryption (or decryption) of block which
is not aligned to cipher block size through userspace crypto
interface, an OOps like this can happen:

[ 112.738285] BUG: unable to handle kernel paging request at e1c44840
[ 112.738407] IP: [<c121f473>] scatterwalk_done+0x53/0x70
...
[ 112.740515] Call Trace:
[ 112.740588] [<c1221d30>] blkcipher_walk_done+0x160/0x1e0
[ 112.740663] [<c12220c8>] blkcipher_walk_next+0x318/0x3c0
[ 112.740737] [<c12221e0>] blkcipher_walk_first+0x70/0x160
[ 112.740811] [<c1222327>] blkcipher_walk_virt+0x17/0x20
[ 112.740886] [<e0ce4249>] cbc_encrypt+0x29/0x100 [aesni_intel]
[ 112.740968] [<c1029f73>] ? get_user_pages_fast+0x123/0x150
[ 112.741046] [<c106e7db>] ? trace_hardirqs_on+0xb/0x10
[ 112.741119] [<e081e1c9>] __ablk_encrypt+0x39/0x40 [ablk_helper]
[ 112.741198] [<e081e1ea>] ablk_encrypt+0x1a/0x70 [ablk_helper]
[ 112.741275] [<e0f715ac>] skcipher_recvmsg+0x20c/0x400 [algif_skcipher]
[ 112.741359] [<c1056a1d>] ? sched_clock_cpu+0x11d/0x1a0
[ 112.741435] [<c10a5eb9>] ? find_get_page+0x79/0xc0
[ 112.741509] [<c135e034>] sock_aio_read+0x104/0x140
[ 112.741580] [<c10be638>] ? __do_fault+0x248/0x420
[ 112.741650] [<c10d3d27>] do_sync_read+0x97/0xd0
[ 112.741719] [<c10d45ed>] vfs_read+0x11d/0x140
[ 112.741789] [<c135f683>] ? sys_socketcall+0x2a3/0x320
[ 112.741861] [<c10d4762>] sys_read+0x42/0x90
[ 112.742578] [<c141c27a>] sysenter_do_call+0x12/0x32

Patch fixes it by simply rejecting buffer which is not multiple of cipher block.

(Bug is present in all stable kernels as well.)

Signed-off-by: Milan Broz <[email protected]>
---
crypto/algif_skcipher.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6a6dfc0..5f7713b 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -463,7 +463,7 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
used -= used % bs;

err = -EINVAL;
- if (!used)
+ if (!used || used % bs)
goto free;

ablkcipher_request_set_crypt(&ctx->req, sg,
--
1.7.10.4


2013-04-14 16:17:40

by Milan Broz

[permalink] [raw]
Subject: Re: [PATCH] algif_skcipher: Avoid crash if buffer is not multiple of cipher block size

On 04/14/2013 06:12 PM, Milan Broz wrote:
> When user requests encryption (or decryption) of block which
> is not aligned to cipher block size through userspace crypto
> interface, an OOps like this can happen

And this is a reproducer for the problem above...

Milan

/*
* Check for unaligned buffer to block cipher size in kernel crypto API
* fixed by patch: http://article.gmane.org/gmane.linux.kernel.cryptoapi/7980
*
* Compile with gcc test.c -o tst
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <inttypes.h>
#include <errno.h>
#include <unistd.h>
#include <sys/socket.h>
#include <linux/if_alg.h>

#ifndef AF_ALG
#define AF_ALG 38
#endif
#ifndef SOL_ALG
#define SOL_ALG 279
#endif

static int kernel_crypt(int opfd, const char *in, char *out, size_t length,
const char *iv, size_t iv_length, uint32_t direction)
{
int r = 0;
ssize_t len;
struct af_alg_iv *alg_iv;
struct cmsghdr *header;
uint32_t *type;
struct iovec iov = {
.iov_base = (void*)(uintptr_t)in,
.iov_len = length,
};
int iv_msg_size = iv ? CMSG_SPACE(sizeof(*alg_iv) + iv_length) : 0;
char buffer[CMSG_SPACE(sizeof(type)) + iv_msg_size];
struct msghdr msg = {
.msg_control = buffer,
.msg_controllen = sizeof(buffer),
.msg_iov = &iov,
.msg_iovlen = 1,
};

if (!in || !out || !length)
return -EINVAL;

if ((!iv && iv_length) || (iv && !iv_length))
return -EINVAL;

memset(buffer, 0, sizeof(buffer));

/* Set encrypt/decrypt operation */
header = CMSG_FIRSTHDR(&msg);
header->cmsg_level = SOL_ALG;
header->cmsg_type = ALG_SET_OP;
header->cmsg_len = CMSG_LEN(sizeof(type));
type = (void*)CMSG_DATA(header);
*type = direction;

/* Set IV */
if (iv) {
header = CMSG_NXTHDR(&msg, header);
header->cmsg_level = SOL_ALG;
header->cmsg_type = ALG_SET_IV;
header->cmsg_len = iv_msg_size;
alg_iv = (void*)CMSG_DATA(header);
alg_iv->ivlen = iv_length;
memcpy(alg_iv->iv, iv, iv_length);
}

len = sendmsg(opfd, &msg, 0);
if (len != (ssize_t)length) {
r = -EIO;
goto bad;
}

len = read(opfd, out, length);
if (len != (ssize_t)length)
r = -EIO;
bad:
memset(buffer, 0, sizeof(buffer));
return r;
}

int main (int argc, char *argv[])
{
const char key[32] = "0123456789abcdef0123456789abcdef";
const char iv[16] = "0000000000000001";
struct sockaddr_alg sa = {
.salg_family = AF_ALG,
.salg_type = "skcipher",
.salg_name = "cbc(aes)"
};
int tfmfd, opfd;
char *data;

if (posix_memalign((void*)&data, 4096, 32)) {
printf("Cannot alloc memory.\n");
return 1;
}

tfmfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
if (tfmfd == -1)
goto bad;

if (bind(tfmfd, (struct sockaddr *)&sa, sizeof(sa)) == -1)
goto bad;

opfd = accept(tfmfd, NULL, 0);
if (opfd == -1)
goto bad;

if (setsockopt(tfmfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key)) == -1)
goto bad;

if (kernel_crypt(opfd, data, data, 1, iv, sizeof(iv), ALG_OP_ENCRYPT) < 0)
printf("Cannot encrypt data.\n");

close(tfmfd);
close(opfd);
free(data);
return 0;
bad:
printf("Cannot initialise cipher.\n");
return 1;
}