From: Jarod Wilson <jarod@redhat.com> Date: Mon, 1 Jun 2009 16:33:42 -0400 Subject: [crypto] testmgr: dynamically allocate xbuf and axbuf Message-id: 200906011633.42227.jarod@redhat.com O-Subject: [RHEL5.4 PATCH 1/2] crypto: testmgr - dynamically allocate xbuf and axbuf Bugzilla: 503091 Bug #503091: [RHEL5.4] Testmanager in crypto API directory may cause data corruption https://bugzilla.redhat.com/show_bug.cgi?id=503091 Backport of http://git.kernel.org/?p=linux/kernel/git/herbert/cryptodev-2.6.git;a=commitdiff;h=ffac89b79ddb0cc5e0ac870e4bcb855ec51804aa ---- crypto: testmgr - Dynamically allocate xbuf and axbuf Herbert Xu [Wed, 6 May 2009 06:15:47 +0000 (14:15 +0800)] We currently allocate temporary memory that is used for testing statically. This renders the testing engine non-reentrant. As algorithms may nest, i.e., one may construct another in order to carry out a part of its operation, this is unacceptable. For example, it has been reported that an AEAD implementation allocates a cipher in its setkey function, which causes it to fail during testing as the temporary memory is overwritten. This patch replaces the static memory with dynamically allocated buffers. We need a maximum of 16 pages so this slightly increases the chances of an algorithm failing due to memory shortage. However, as testing usually occurs at registration, this shouldn't be a big problem. Reported-by: Shasi Pulijala <spulijala@amcc.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> ---- RHEL5 backport tested by 'modprobe tcrypt' (run all crypto self-tests) and fips cavs test vector stacks, all tests pass w/o issue. diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 5b19049..9537adb 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -112,9 +112,6 @@ struct crypto_test_param { static unsigned int IDX[8] = { IDX1, IDX2, IDX3, IDX4, IDX5, IDX6, IDX7, IDX8 }; -static char *xbuf[XBUFSIZE]; -static char *axbuf[XBUFSIZE]; - static void hexdump(unsigned char *buf, unsigned int len) { while (len--) @@ -134,6 +131,33 @@ static void tcrypt_complete(struct crypto_async_request *req, int err) complete(&res->completion); } +static int testmgr_alloc_buf(char *buf[XBUFSIZE]) +{ + int i; + + for (i = 0; i < XBUFSIZE; i++) { + buf[i] = (void *)__get_free_page(GFP_KERNEL); + if (!buf[i]) + goto err_free_buf; + } + + return 0; + +err_free_buf: + while (i-- > 0) + free_page((unsigned long)buf[i]); + + return -ENOMEM; +} + +static void testmgr_free_buf(char *buf[XBUFSIZE]) +{ + int i; + + for (i = 0; i < XBUFSIZE; i++) + free_page((unsigned long)buf[i]); +} + static int test_nhash(struct crypto_hash *tfm, struct hash_testvec *template, unsigned int tcount) { @@ -142,8 +166,12 @@ static int test_nhash(struct crypto_hash *tfm, struct hash_testvec *template, struct scatterlist sg[8]; char result[64]; struct hash_desc desc; - int ret; void *hash_buff; + char *xbuf[XBUFSIZE]; + int ret = -ENOMEM; + + if (testmgr_alloc_buf(xbuf)) + goto out_nobuf; desc.tfm = tfm; desc.flags = 0; @@ -237,6 +265,8 @@ static int test_nhash(struct crypto_hash *tfm, struct hash_testvec *template, ret = 0; +out_nobuf: + testmgr_free_buf(xbuf); out: return ret; } @@ -246,7 +276,7 @@ static int test_aead(struct crypto_aead *tfm, int enc, { const char *algo = ncrypto_tfm_alg_driver_name(crypto_aead_tfm(tfm)); unsigned int i, j, k, n, temp; - int ret = 0; + int ret = -ENOMEM; char *q; char *key; struct aead_request *req; @@ -258,6 +288,13 @@ static int test_aead(struct crypto_aead *tfm, int enc, void *input; void *assoc; char iv[MAX_IVLEN]; + char *xbuf[XBUFSIZE]; + char *axbuf[XBUFSIZE]; + + if (testmgr_alloc_buf(xbuf)) + goto out_noxbuf; + if (testmgr_alloc_buf(axbuf)) + goto out_noaxbuf; if (enc == ENCRYPT) e = "encryption"; @@ -270,7 +307,6 @@ static int test_aead(struct crypto_aead *tfm, int enc, if (!req) { printk(KERN_ERR "alg: aead: Failed to allocate request for " "%s\n", algo); - ret = -ENOMEM; goto out; } @@ -550,6 +586,10 @@ static int test_aead(struct crypto_aead *tfm, int enc, out: aead_request_free(req); + testmgr_free_buf(axbuf); +out_noaxbuf: + testmgr_free_buf(xbuf); +out_noxbuf: return ret; } @@ -558,10 +598,14 @@ static int test_cipher(struct crypto_cipher *tfm, int enc, { const char *algo = crypto_tfm_alg_driver_name(crypto_cipher_tfm(tfm)); unsigned int i, j, k; - int ret; char *q; const char *e; void *data; + char *xbuf[XBUFSIZE]; + int ret = -ENOMEM; + + if (testmgr_alloc_buf(xbuf)) + goto out_nobuf; if (enc == ENCRYPT) e = "encryption"; @@ -615,6 +659,8 @@ static int test_cipher(struct crypto_cipher *tfm, int enc, ret = 0; out: + testmgr_free_buf(xbuf); +out_nobuf: return ret; } @@ -624,7 +670,6 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc, const char *algo = ncrypto_tfm_alg_driver_name(crypto_ablkcipher_tfm(tfm)); unsigned int i, j, k, n, temp; - int ret; char *q; struct ablkcipher_request *req; struct scatterlist sg[8]; @@ -632,6 +677,11 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc, struct tcrypt_result result; void *data; char iv[MAX_IVLEN]; + char *xbuf[XBUFSIZE]; + int ret = -ENOMEM; + + if (testmgr_alloc_buf(xbuf)) + goto out_nobuf; if (enc == ENCRYPT) e = "encryption"; @@ -644,7 +694,6 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc, if (!req) { printk(KERN_ERR "alg: skcipher: Failed to allocate request " "for %s\n", algo); - ret = -ENOMEM; goto out; } @@ -831,6 +880,8 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc, out: ablkcipher_request_free(req); + testmgr_free_buf(xbuf); +out_nobuf: return ret; } @@ -1633,49 +1684,15 @@ static struct notifier_block testmgr_notifier = { static int __init testmgr_init(void) { - int err = -ENOMEM; - int i; - - for (i = 0; i < XBUFSIZE; i++) { - xbuf[i] = (void *)__get_free_page(GFP_KERNEL); - if (!xbuf[i]) - goto err_free_xbuf; - } - - for (i = 0; i < XBUFSIZE; i++) { - axbuf[i] = (void *)__get_free_page(GFP_KERNEL); - if (!axbuf[i]) - goto err_free_axbuf; - } - - err = crypto_register_notifier(&testmgr_notifier); - if (err) - goto err_free_axbuf; - - return 0; - -err_free_axbuf: - for (i = 0; i < XBUFSIZE && axbuf[i]; i++) - free_page((unsigned long)axbuf[i]); -err_free_xbuf: - for (i = 0; i < XBUFSIZE && xbuf[i]; i++) - free_page((unsigned long)xbuf[i]); - - return err; + return crypto_register_notifier(&testmgr_notifier); } static void __exit testmgr_exit(void) { int err; - int i; err = crypto_unregister_notifier(&testmgr_notifier); BUG_ON(err); - - for (i = 0; i < XBUFSIZE; i++) - free_page((unsigned long)axbuf[i]); - for (i = 0; i < XBUFSIZE; i++) - free_page((unsigned long)xbuf[i]); } subsys_initcall(testmgr_init);