[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161209230851.GB64048@google.com>
Date: Fri, 9 Dec 2016 15:08:51 -0800
From: Eric Biggers <ebiggers3@...il.com>
To: linux-crypto@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
kernel-hardening@...ts.openwall.com,
Herbert Xu <herbert@...dor.apana.org.au>,
Andrew Lutomirski <luto@...nel.org>,
Stephan Mueller <smueller@...onox.de>
Subject: Remaining crypto API regressions with CONFIG_VMAP_STACK
In the 4.9 kernel, virtually-mapped stacks will be supported and enabled by
default on x86_64. This has been exposing a number of problems in which
on-stack buffers are being passed into the crypto API, which to support crypto
accelerators operates on 'struct page' rather than on virtual memory.
Some of these problems have already been fixed, but I was wondering how many
problems remain, so I briefly looked through all the callers of sg_set_buf() and
sg_init_one(). Overall I found quite a few remaining problems, detailed below.
The following crypto drivers initialize a scatterlist to point into an
ahash_request, which may have been allocated on the stack with
AHASH_REQUEST_ON_STACK():
drivers/crypto/bfin_crc.c:351
drivers/crypto/qce/sha.c:299
drivers/crypto/sahara.c:973,988
drivers/crypto/talitos.c:1910
drivers/crypto/ccp/ccp-crypto-aes-cmac.c:105,119,142
drivers/crypto/ccp/ccp-crypto-sha.c:95,109,124
drivers/crypto/qce/sha.c:325
The following crypto drivers initialize a scatterlist to point into an
ablkcipher_request, which may have been allocated on the stack with
SKCIPHER_REQUEST_ON_STACK():
drivers/crypto/ccp/ccp-crypto-aes-xts.c:162
drivers/crypto/ccp/ccp-crypto-aes.c:94
And these other places do crypto operations on buffers clearly on the stack:
drivers/net/wireless/intersil/orinoco/mic.c:72
drivers/usb/wusbcore/crypto.c:264
net/ceph/crypto.c:182
net/rxrpc/rxkad.c:737,1000
security/keys/encrypted-keys/encrypted.c:500
fs/cifs/smbencrypt.c:96
Note: I almost certainly missed some, since I excluded places where the use of a
stack buffer was not obvious to me. I also excluded AEAD algorithms since there
isn't an AEAD_REQUEST_ON_STACK() macro (yet).
The "good" news with these bugs is that on x86_64 without CONFIG_DEBUG_SG=y or
CONFIG_DEBUG_VIRTUAL=y, you can still do virt_to_page() and then page_address()
on a vmalloc address and get back the same address, even though you aren't
*supposed* to be able to do this. This will make things still work for most
people. The bad news is that if you happen to have consumed just about 1 page
(or N pages) of your stack at the time you call the crypto API, your stack
buffer may actually span physically non-contiguous pages, so the crypto
algorithm will scribble over some unrelated page. Also, hardware crypto drivers
which actually do operate on physical memory will break too.
So I am wondering: is the best solution really to make all these crypto API
algorithms and users use heap buffers, as opposed to something like maintaining
a lowmem alias for the stack, or introducing a more general function to convert
buffers (possibly in the vmalloc space) into scatterlists? And if the current
solution is desired, who is going to fix all of these bugs and when?
Eric
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
