linux-kernel - Doing crypto in small stack buffers (bluetooth vs vmalloc-stack crash, etc)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALCETrVJzzKkRsSbgsUmVd_+ArKEgoRSVdG1tVp7CXzFoPyVgA@mail.gmail.com>
Date:	Tue, 21 Jun 2016 10:43:40 -0700
From:	Andy Lutomirski <luto@...nel.org>
To:	linux-bluetooth@...r.kernel.org,
	Johan Hedberg <johan.hedberg@...il.com>,
	Gustavo Padovan <gustavo@...ovan.org>,
	Marcel Holtmann <marcel@...tmann.org>,
	linux-crypto@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Doing crypto in small stack buffers (bluetooth vs vmalloc-stack
 crash, etc)

net/bluetooth/smp.c (in smp_e) wants to do AES-ECB on a 16-byte stack
buffer, which seems eminently reasonable to me.  It does it like this:

    sg_init_one(&sg, data, 16);

    skcipher_request_set_tfm(req, tfm);
    skcipher_request_set_callback(req, 0, NULL, NULL);
    skcipher_request_set_crypt(req, &sg, &sg, 16, NULL);

    err = crypto_skcipher_encrypt(req);
    skcipher_request_zero(req);
    if (err)
        BT_ERR("Encrypt data error %d", err);

I tried to figure out what exactly that does, and I got like in so
many layers of indirection that I mostly gave up.  But it appears to
map the stack address to a physical address, stick it in a
scatterlist, follow several function pointers, go through a bunch of
"scatterwalk" indirection, and call blkcipher_next_fast, which calls
blkcipher_map_src, which calls scatterwalk_map, which calls
kmap_atomic (!).  This is anything but fast.

I think this code has several serious problems:

 - It uses kmap_atomic to access a 16-byte stack buffer.  This is absurd.

 - It blows up if the stack is in vmalloc space, because you can't
virt_to_phys on the stack buffer in the first place.  (This is why I
care.)  And I really, really don't want to write sg_init_stack to
create a scatterlist that points to the stack, although such a thing
could be done if absolutely necessary.

 - It's very, very compllicated and it does something very, very
simple (call aes_encrypt on a plain old u8 *).

Oh yeah, it sets CRYPTO_ALG_ASYNC, too.  I can't even figure out what
that does because the actual handling of that flag is so many layers
deep.

Is there a straightforward way that bluetooth and, potentially, other
drivers can just do synchronous crypto in a small buffer specified by
its virtual address?  The actual cryptography part of the crypto code
already works this way, but I can't find an API for it.

--Andy