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

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

[Milan Broz]

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 <gmazyland@gmail.com>
---
 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

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

[Milan Broz]

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;
}

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

[Milan Broz]

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

[   37.672185] BUG: unable to handle kernel paging request at e1c5fbc0
[   37.672457] IP: [<c1214393>] scatterwalk_done+0x53/0x70
...
[   37.679183] Call Trace:
[   37.679326]  [<c1216cf0>] blkcipher_walk_done+0x160/0x1e0
[   37.679516]  [<c1217088>] blkcipher_walk_next+0x318/0x3c0
[   37.679707]  [<c1007f1c>] ? native_sched_clock+0x2c/0xb0
[   37.679895]  [<c12171a0>] blkcipher_walk_first+0x70/0x160
[   37.680082]  [<c12172e7>] blkcipher_walk_virt+0x17/0x20
[   37.680272]  [<e0a36249>] cbc_encrypt+0x29/0x100 [aesni_intel]
[   37.680472]  [<c1029403>] ? get_user_pages_fast+0x123/0x150
[   37.680665]  [<c106e9fb>] ? trace_hardirqs_on+0xb/0x10
[   37.680851]  [<e083a1c9>] __ablk_encrypt+0x39/0x40 [ablk_helper]
[   37.681052]  [<e083a1ea>] ablk_encrypt+0x1a/0x70 [ablk_helper]
[   37.681250]  [<e0de95ac>] skcipher_recvmsg+0x20c/0x400 [algif_skcipher]
[   37.681474]  [<c1346897>] ? sock_kfree_s+0x17/0x30
[   37.681652]  [<c1344bc4>] sock_aio_read+0x104/0x140
[   37.681834]  [<c10d2637>] do_sync_read+0x97/0xd0
[   37.682009]  [<c10d2f1d>] vfs_read+0x11d/0x140
[   37.682183]  [<c1346213>] ? sys_socketcall+0x2a3/0x320
[   37.682366]  [<c10d2f82>] sys_read+0x42/0x90
[   37.682597]  [<c1403afa>] sysenter_do_call+0x12/0x32

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

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 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