Skip to main content

Browse the archive

Revision a318f12ed8843cfac53198390c74a565c632f417 authored by Kees Cook on 16 July 2019, 23:30:21 UTC, committed by Linus Torvalds on 17 July 2019, 02:23:24 UTC 
ipc/mqueue.c: only perform resource calculation if user valid
 
Andreas Christoforou reported:

  UBSAN: Undefined behaviour in ipc/mqueue.c:414:49 signed integer overflow:
  9 * 2305843009213693951 cannot be represented in type 'long int'
  ...
  Call Trace:
    mqueue_evict_inode+0x8e7/0xa10 ipc/mqueue.c:414
    evict+0x472/0x8c0 fs/inode.c:558
    iput_final fs/inode.c:1547 [inline]
    iput+0x51d/0x8c0 fs/inode.c:1573
    mqueue_get_inode+0x8eb/0x1070 ipc/mqueue.c:320
    mqueue_create_attr+0x198/0x440 ipc/mqueue.c:459
    vfs_mkobj+0x39e/0x580 fs/namei.c:2892
    prepare_open ipc/mqueue.c:731 [inline]
    do_mq_open+0x6da/0x8e0 ipc/mqueue.c:771

Which could be triggered by:

        struct mq_attr attr = {
                .mq_flags = 0,
                .mq_maxmsg = 9,
                .mq_msgsize = 0x1fffffffffffffff,
                .mq_curmsgs = 0,
        };

        if (mq_open("/testing", 0x40, 3, &attr) == (mqd_t) -1)
                perror("mq_open");

mqueue_get_inode() was correctly rejecting the giant mq_msgsize, and
preparing to return -EINVAL.  During the cleanup, it calls
mqueue_evict_inode() which performed resource usage tracking math for
updating "user", before checking if there was a valid "user" at all
(which would indicate that the calculations would be sane).  Instead,
delay this check to after seeing a valid "user".

The overflow was real, but the results went unused, so while the flaw is
harmless, it's noisy for kernel fuzzers, so just fix it by moving the
calculation under the non-NULL "user" where it actually gets used.

Link: http://lkml.kernel.org/r/201906072207.ECB65450@keescook
Signed-off-by: Kees Cook <keescook@chromium.org>
Reported-by: Andreas Christoforou <andreaschristofo@gmail.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent 6b15f67
Raw File
xxhash_generic.c 
// SPDX-License-Identifier: GPL-2.0

#include <crypto/internal/hash.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/xxhash.h>
#include <asm/unaligned.h>

#define XXHASH64_BLOCK_SIZE	32
#define XXHASH64_DIGEST_SIZE	8

struct xxhash64_tfm_ctx {
	u64 seed;
};

struct xxhash64_desc_ctx {
	struct xxh64_state xxhstate;
};

static int xxhash64_setkey(struct crypto_shash *tfm, const u8 *key,
			 unsigned int keylen)
{
	struct xxhash64_tfm_ctx *tctx = crypto_shash_ctx(tfm);

	if (keylen != sizeof(tctx->seed)) {
		crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
		return -EINVAL;
	}
	tctx->seed = get_unaligned_le64(key);
	return 0;
}

static int xxhash64_init(struct shash_desc *desc)
{
	struct xxhash64_tfm_ctx *tctx = crypto_shash_ctx(desc->tfm);
	struct xxhash64_desc_ctx *dctx = shash_desc_ctx(desc);

	xxh64_reset(&dctx->xxhstate, tctx->seed);

	return 0;
}

static int xxhash64_update(struct shash_desc *desc, const u8 *data,
			 unsigned int length)
{
	struct xxhash64_desc_ctx *dctx = shash_desc_ctx(desc);

	xxh64_update(&dctx->xxhstate, data, length);

	return 0;
}

static int xxhash64_final(struct shash_desc *desc, u8 *out)
{
	struct xxhash64_desc_ctx *dctx = shash_desc_ctx(desc);

	put_unaligned_le64(xxh64_digest(&dctx->xxhstate), out);

	return 0;
}

static int xxhash64_digest(struct shash_desc *desc, const u8 *data,
			 unsigned int length, u8 *out)
{
	struct xxhash64_tfm_ctx *tctx = crypto_shash_ctx(desc->tfm);

	put_unaligned_le64(xxh64(data, length, tctx->seed), out);

	return 0;
}

static struct shash_alg alg = {
	.digestsize	= XXHASH64_DIGEST_SIZE,
	.setkey		= xxhash64_setkey,
	.init		= xxhash64_init,
	.update		= xxhash64_update,
	.final		= xxhash64_final,
	.digest		= xxhash64_digest,
	.descsize	= sizeof(struct xxhash64_desc_ctx),
	.base		= {
		.cra_name	 = "xxhash64",
		.cra_driver_name = "xxhash64-generic",
		.cra_priority	 = 100,
		.cra_flags	 = CRYPTO_ALG_OPTIONAL_KEY,
		.cra_blocksize	 = XXHASH64_BLOCK_SIZE,
		.cra_ctxsize	 = sizeof(struct xxhash64_tfm_ctx),
		.cra_module	 = THIS_MODULE,
	}
};

static int __init xxhash_mod_init(void)
{
	return crypto_register_shash(&alg);
}

static void __exit xxhash_mod_fini(void)
{
	crypto_unregister_shash(&alg);
}

subsys_initcall(xxhash_mod_init);
module_exit(xxhash_mod_fini);

MODULE_AUTHOR("Nikolay Borisov <nborisov@suse.com>");
MODULE_DESCRIPTION("xxhash calculations wrapper for lib/xxhash.c");
MODULE_LICENSE("GPL");
MODULE_ALIAS_CRYPTO("xxhash64");
MODULE_ALIAS_CRYPTO("xxhash64-generic");
back to top