RE: [PATCH] omap-aes - fix crypto cleanup
Hi Tero, > Also, I think this patch should be split up in two, as there are two > issues you are fixing; the bad pointer issue (which I think you only > fixed partially, also the in->sgl has similar problem), and the missing > output IVI. Why is this needed btw, I have never faced the requirement > to update the IVI on the ahash request? Any crypto test cases I have ran > just work fine without this bit. I spotted the in_sgl issue also, the code looks very suspicious and should. be reworked. For the moment the bad kfree from in_sgl has never occurred. I'll recheck and retest. I'll send a separate patch for the IV case Signed-off-by: Francis Le Bourse --- drivers/crypto/omap-aes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c index 49bd56f..1e040eb 100644 --- a/drivers/crypto/omap-aes.c +++ b/drivers/crypto/omap-aes.c @@ -498,7 +498,7 @@ static void omap_aes_done_task(unsigned long data) omap_crypto_cleanup(dd->in_sgl, NULL, 0, dd->total_save, FLAGS_IN_DATA_ST_SHIFT, dd->flags); - omap_crypto_cleanup(&dd->out_sgl, dd->orig_out, 0, dd->total_save, + omap_crypto_cleanup(dd->out_sg, dd->orig_out, 0, dd->total_save, FLAGS_OUT_DATA_ST_SHIFT, dd->flags); omap_aes_finish_req(dd, 0);
Re: [PATCH] omap-aes - fix crypto cleanup and IV reporting
Hi Francis, This has similar checkpatch issues + being split into multipart message as your other patch. Also, I think this patch should be split up in two, as there are two issues you are fixing; the bad pointer issue (which I think you only fixed partially, also the in->sgl has similar problem), and the missing output IVI. Why is this needed btw, I have never faced the requirement to update the IVI on the ahash request? Any crypto test cases I have ran just work fine without this bit. -Tero On 22/03/18 16:58, Francis Le Bourse wrote: Hello, omap_aes_(cbc/ctr)(encrypt/decrypt) don't return the updated IV, add the code to do just that at the end of the operation. In omap_aes_done_task(), omap_crypto_cleanup() is called with: omap_crypto_cleanup(&dd->out_sgl, dd->orig_out, 0, dd->total_save, FLAGS_OUT_DATA_ST_SHIFT, dd->flags); this ends up in freeing the part of the omap_aes_dev structure starting at the address of the out_sgl member. As the memory released is soon reused the kernel crashes at the next encrypt/decrypt call: [ 334.559643] Unable to handle kernel paging request at virtual address 30627375 [ 334.566922] pgd = c0004000 [ 334.569650] [30627375] *pgd= [ 334.573252] Internal error: Oops: 5 [#1] SMP ARM Entering kdb (current=0xee5fb0c0, pid 89) on processor 1 Oops: (null) due to oops @ 0xc0b36a20 CPU: 1 PID: 89 Comm: 4b50.aes-en Tainted: G C 4.14.13-ti-r25 #55 Hardware name: Generic DRA74X (Flattened Device Tree) task: ee5fb0c0 task.stack: ee782000 PC is at omap_aes_crypt_dma+0x234/0x58c LR is at irq_work_queue+0x14/0x90 pc : [] lr : [] psr: 60080013 sp : ee783e48 ip : 0007 fp : ee783eac r10: eba7f740 r9 : eba7f740 r8 : ee70d380 r7 : 0001 r6 : c011a320 r5 : c1504dc8 r4 : ee70d310 r3 : 5c40a269 r2 : 5c40a269 r1 : r0 : 30627375 Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: ad69c06a DAC: 0051 CPU: 1 PID: 89 Comm: 4b50.aes-en Tainted: G C 4.14.13-ti-r25 #55 Hardware name: Generic DRA74X (Flattened Device Tree) [] (__dabt_svc) from [] (omap_aes_crypt_dma+0x234/0x58c) [] (omap_aes_crypt_dma) from [] (omap_aes_crypt_dma_start+0x228/0x400) [] (omap_aes_crypt_dma_start) from [] (omap_aes_crypt_req+0x94/0x128) [] (omap_aes_crypt_req) from [] (crypto_pump_work+0x278/0x2f8) [] (crypto_pump_work) from [] (kthread_worker_fn+0x11c/0x20c) [] (kthread_worker_fn) from [] (kthread+0x170/0x178) [] (kthread) from [] (ret_from_fork+0x14/0x2c) Here the call should be omap_crypt_cleanup(dd->out_sg, ...); A similar issue exists in omap-des.c. -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
