From 161613293fd4b7d5ceb1faab788f47e688e07a67 Mon Sep 17 00:00:00 2001 From: Patrick McHardy Date: Tue, 29 Apr 2008 21:44:28 +0800 Subject: [PATCH 1/4] [CRYPTO] authenc: Fix async crypto crash in crypto_authenc_genicv() crypto_authenc_givencrypt_done uses req->data as struct aead_givcrypt_request, while it really points to a struct aead_request, causing this crash: BUG: unable to handle kernel paging request at 6b6b6b6b IP: [] :authenc:crypto_authenc_genicv+0x23/0x109 *pde = 00000000 Oops: 0000 [#1] PREEMPT DEBUG_PAGEALLOC Modules linked in: hifn_795x authenc esp4 aead xfrm4_mode_tunnel sha1_generic hmac crypto_hash] Pid: 3074, comm: ping Not tainted (2.6.25 #4) EIP: 0060:[] EFLAGS: 00010296 CPU: 0 EIP is at crypto_authenc_genicv+0x23/0x109 [authenc] EAX: daa04690 EBX: daa046e0 ECX: dab0a100 EDX: daa046b0 ESI: 6b6b6b6b EDI: dc872054 EBP: c033ff60 ESP: c033ff0c DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068 Process ping (pid: 3074, ti=c033f000 task=db883a80 task.ti=dab6c000) Stack: 00000000 daa046b0 c0215a3e daa04690 dab0a100 00000000 ffffffff db9fd7f0 dba208c0 dbbb1720 00000001 daa04720 00000001 c033ff54 c0119ca9 dc852a75 c033ff60 c033ff60 daa046e0 00000000 00000001 c033ff6c dc87527b 00000001 Call Trace: [] ? dev_alloc_skb+0x14/0x29 [] ? printk+0x15/0x17 [] ? crypto_authenc_givencrypt_done+0x1a/0x27 [authenc] [] ? hifn_process_ready+0x34a/0x352 [hifn_795x] [] ? rhine_napipoll+0x3f2/0x3fd [via_rhine] [] ? hifn_check_for_completion+0x4d/0xa6 [hifn_795x] [] ? hifn_tasklet_callback+0xa/0xc [hifn_795x] [] ? tasklet_action+0x3f/0x66 [] ? __do_softirq+0x38/0x7a [] ? do_softirq+0x3e/0x71 [] ? irq_exit+0x2c/0x65 [] ? smp_apic_timer_interrupt+0x5f/0x6a [] ? apic_timer_interrupt+0x28/0x30 [] ? hifn_handle_req+0x44a/0x50d [hifn_795x] ... Signed-off-by: Patrick McHardy Signed-off-by: Herbert Xu --- crypto/authenc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crypto/authenc.c b/crypto/authenc.c index ed8ac5a6fa5..4b226768752 100644 --- a/crypto/authenc.c +++ b/crypto/authenc.c @@ -217,9 +217,10 @@ static void crypto_authenc_givencrypt_done(struct crypto_async_request *req, int err) { if (!err) { - struct aead_givcrypt_request *greq = req->data; + struct aead_request *areq = req->data; + struct skcipher_givcrypt_request *greq = aead_request_ctx(areq); - err = crypto_authenc_genicv(&greq->areq, greq->giv, 0); + err = crypto_authenc_genicv(areq, greq->giv, 0); } aead_request_complete(req->data, err); From 8ec970d8561abb5645d4602433b772e268c96d05 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Tue, 29 Apr 2008 21:53:52 +0800 Subject: [PATCH 2/4] [CRYPTO] api: Fix scatterwalk_sg_chain When I backed out of using the generic sg chaining (as it isn't currently portable) and introduced scatterwalk_sg_chain/scatterwalk_sg_next I left out the sg_is_last check in the latter. This causes it to potentially dereference beyond the end of the sg array. As most uses of scatterwalk_sg_next are bound by an overall length, this only affected the chaining code in authenc and eseqiv. Thanks to Patrick McHardy for identifying this problem. This patch also clears the "last" bit on the head of the chained list as it's no longer last. This also went missing in scatterwalk_sg_chain and is present in sg_chain. Signed-off-by: Herbert Xu --- include/crypto/scatterwalk.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/crypto/scatterwalk.h b/include/crypto/scatterwalk.h index 224658b8d80..833d208c25d 100644 --- a/include/crypto/scatterwalk.h +++ b/include/crypto/scatterwalk.h @@ -57,10 +57,14 @@ static inline void scatterwalk_sg_chain(struct scatterlist *sg1, int num, struct scatterlist *sg2) { sg_set_page(&sg1[num - 1], (void *)sg2, 0, 0); + sg1[num - 1].page_link &= ~0x02; } static inline struct scatterlist *scatterwalk_sg_next(struct scatterlist *sg) { + if (sg_is_last(sg)) + return NULL; + return (++sg)->length ? sg : (void *)sg_page(sg); } From 46f8153cc59384eb09a426d044668d4801f818ce Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Tue, 29 Apr 2008 21:57:01 +0800 Subject: [PATCH 3/4] [CRYPTO] eseqiv: Fix off-by-one encryption After attaching the IV to the head during encryption, eseqiv does not increase the encryption length by that amount. As such the last block of the actual plain text will be left unencrypted. Fortunately the only user of this code hifn currently crashes so this shouldn't affect anyone :) Signed-off-by: Herbert Xu --- crypto/eseqiv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crypto/eseqiv.c b/crypto/eseqiv.c index b14f14e314b..881d3091043 100644 --- a/crypto/eseqiv.c +++ b/crypto/eseqiv.c @@ -136,7 +136,8 @@ static int eseqiv_givencrypt(struct skcipher_givcrypt_request *req) } ablkcipher_request_set_crypt(subreq, reqctx->src, dst, - req->creq.nbytes, req->creq.info); + req->creq.nbytes + ivsize, + req->creq.info); memcpy(req->creq.info, ctx->salt, ivsize); From b1145ce395f7785487c128fe8faf8624e6586d84 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Wed, 30 Apr 2008 00:27:14 +0800 Subject: [PATCH 4/4] [CRYPTO] cryptd: Correct kzalloc error test Normally, kzalloc returns NULL or a valid pointer value, not a value to be tested using IS_ERR. Signed-off-by: Julia Lawall Signed-off-by: Herbert Xu --- crypto/cryptd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crypto/cryptd.c b/crypto/cryptd.c index 250425263e0..b150de56205 100644 --- a/crypto/cryptd.c +++ b/crypto/cryptd.c @@ -190,8 +190,10 @@ static struct crypto_instance *cryptd_alloc_instance(struct crypto_alg *alg, int err; inst = kzalloc(sizeof(*inst) + sizeof(*ctx), GFP_KERNEL); - if (IS_ERR(inst)) + if (!inst) { + inst = ERR_PTR(-ENOMEM); goto out; + } err = -ENAMETOOLONG; if (snprintf(inst->alg.cra_driver_name, CRYPTO_MAX_ALG_NAME,