[PATCH] crypto: talitos - Fix panic in interrupt error path
The talitos interrupt error path can generate a kernel panic when priv-chan[ch].fifo[tail].desc is NULL. Check the desc pointer before use in current_desc_hdr. Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc01e0f40 Oops: Kernel access of bad area, sig: 11 [#1] SMP NR_CPUS=2 P1020 RDB last sysfs file: /sys/kernel/uevent_seqnum Modules linked in: pl2303 option keyspan sg usb_wwan usbserial uhci_hcd ohci_hcd macvlan ebt_snat ebt_dnat ebt_arpreply ebt_ip ebt_arp ebt_redirect ebt_mark ebt_vlan ebt_stp ebt_pkttype ebt_mark_m ebt_limit ebt_among ebt_802_3 ebtable_nc NIP: c01e0f40 LR: c01e0ea8 CTR: c01e0ccc REGS: efff1ea0 TRAP: 0300 Not tainted (2.6.38.8) MSR: 00021000 ME,CE CR: 99355033 XER: c000 DEAR: , ESR: TASK = c03422e0[0] 'swapper' THREAD: c035a000 CPU: 0 GPR00: efff1f50 c03422e0 ef869608 ef869608 efff1ff0 GPR08: efb6ec80 efb75e00 efb75e10 1007e92c c02f1740 GPR16: c02f c02f16bc c02a ffea 0003 0001 1280 GPR24: c02d78a4 010c0100 efb380c0 002d 000186a0 ef869608 0008 NIP [c01e0f40] talitos_interrupt+0x274/0x8a0 LR [c01e0ea8] talitos_interrupt+0x1dc/0x8a0 Call Trace: [efff1fa0] [c0066aa4] handle_IRQ_event+0x44/0x110 [efff1fd0] [c0069468] handle_fasteoi_irq+0x100/0x168 [efff1ff0] [c000d0ac] call_handle_irq+0x18/0x28 [efff3c30] [c000449c] do_IRQ+0xe8/0x168 [efff3c60] [c000f0a4] ret_from_except+0x0/0x18 --- Exception: 501 at udp_queue_rcv_skb+0xe0/0x378 LR = udp_queue_rcv_skb+0xdc/0x378 [efff3d40] [c024a780] __udp4_lib_rcv+0x31c/0x5cc [efff3d80] [c0223518] ip_local_deliver_finish+0x114/0x27c [efff3da0] [c02233e0] ip_rcv_finish+0x4b0/0x4d4 [efff3dc0] [c01fa390] __netif_receive_skb+0x3f8/0x43c [efff3e10] [c01fab38] netif_receive_skb+0x98/0xac [efff3e40] [c01b84a0] gfar_clean_rx_ring+0x37c/0x470 [efff3ea0] [c01b89cc] gfar_poll+0x438/0x550 [efff3f60] [c01fae20] net_rx_action+0x88/0x170 [efff3fa0] [c0036218] __do_softirq+0xd8/0x170 [efff3ff0] [c000d084] call_do_softirq+0x14/0x24 [c035be60] [c000471c] do_softirq+0x7c/0x9c [c035be80] [c0036364] irq_exit+0x50/0x60 [c035be90] [c00044f8] do_IRQ+0x144/0x168 [c035bec0] [c000f0a4] ret_from_except+0x0/0x18 --- Exception: 501 at cpu_idle+0xc8/0x154 LR = cpu_idle+0xc8/0x154 [c035bf80] [c0008788] cpu_idle+0x150/0x154 (unreliable) [c035bfb0] [c000236c] rest_init+0x68/0x7c [c035bfc0] [c0318858] start_kernel+0x2f4/0x308 [c035bff0] [c3d8] skpinv+0x2f0/0x32c Instruction dump: 7fa3eb78 4bf98945 7c7b1b78 4838 552b2036 39290001 7d6a5a14 800b0004 7f87 409effb0 812b 7fa3eb78 8309 4bf98915 7c7b1b78 2f98 Kernel panic - not syncing: Fatal exception in interrupt Call Trace: [efff1dd0] [c0007bd0] show_stack+0x5c/0x164 (unreliable) [efff1e10] [c028206c] panic+0xa8/0x1d8 [efff1e60] [c000a654] die+0x1f8/0x23c [efff1e80] [c00135c8] bad_page_fault+0x100/0x114 [efff1e90] [c000eefc] handle_page_fault+0x7c/0x80 --- Exception: 300 at talitos_interrupt+0x274/0x8a0 LR = talitos_interrupt+0x1dc/0x8a0 [efff1f50] [] (null) (unreliable) [efff1fa0] [c0066aa4] handle_IRQ_event+0x44/0x110 [efff1fd0] [c0069468] handle_fasteoi_irq+0x100/0x168 [efff1ff0] [c000d0ac] call_handle_irq+0x18/0x28 [efff3c30] [c000449c] do_IRQ+0xe8/0x168 [efff3c60] [c000f0a4] ret_from_except+0x0/0x18 --- Exception: 501 at udp_queue_rcv_skb+0xe0/0x378 LR = udp_queue_rcv_skb+0xdc/0x378 [efff3d40] [c024a780] __udp4_lib_rcv+0x31c/0x5cc [efff3d80] [c0223518] ip_local_deliver_finish+0x114/0x27c [efff3da0] [c02233e0] ip_rcv_finish+0x4b0/0x4d4 [efff3dc0] [c01fa390] __netif_receive_skb+0x3f8/0x43c [efff3e10] [c01fab38] netif_receive_skb+0x98/0xac [efff3e40] [c01b84a0] gfar_clean_rx_ring+0x37c/0x470 [efff3ea0] [c01b89cc] gfar_poll+0x438/0x550 [efff3f60] [c01fae20] net_rx_action+0x88/0x170 [efff3fa0] [c0036218] __do_softirq+0xd8/0x170 [efff3ff0] [c000d084] call_do_softirq+0x14/0x24 [c035be60] [c000471c] do_softirq+0x7c/0x9c [c035be80] [c0036364] irq_exit+0x50/0x60 [c035be90] [c00044f8] do_IRQ+0x144/0x168 [c035bec0] [c000f0a4] ret_from_except+0x0/0x18 --- Exception: 501 at cpu_idle+0xc8/0x154 LR = cpu_idle+0xc8/0x154 [c035bf80] [c0008788] cpu_idle+0x150/0x154 (unreliable) [c035bfb0] [c000236c] rest_init+0x68/0x7c [c035bfc0] [c0318858] start_kernel+0x2f4/0x308 [c035bff0] [c3d8] skpinv+0x2f0/0x32c Signed-off-by: Helmut Schaa helmut.sc...@googlemail.com Cc: Sven Schnelle sv...@stackframe.org Cc: Kim Phillips kim.phill...@freescale.com --- Not sure if this is the same bug as talitos: handle descriptor not found in error path was intended to fix. But I've been running this one for a while now without experiencing the crash anymore. Thanks, Helmut drivers/crypto/talitos.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index 921039e..56d7804 100644 --- a/drivers/crypto/talitos.c +++ b/drivers/crypto/talitos.c @@ -459,7 +459,9 @@
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Wed, May 30, 2012 at 10:30:25AM +0300, Jussi Kivilinna wrote: Quoting Andi Kleen a...@firstfloor.org: The driver needs CPUID annotations now (since 3.3), so that it can be autoloaded. Something like: Is it really good idea to autoload crypto modules? Currently loading different cipher implementations is handled via module-aliases and cipher priorities. With autoloading by CPUID, crypto-modules would be loaded and be mostly unused. For example, serpent-sse2 would be autoloaded on all x86-64 kernels (well, on those that have it compiled as module = probably most of distros). IMHO these should be loaded only when needed, as is done with generic i586/x86-64 assembler cipher implementations. I agree with that. Currently when I boot my PC with a new 3.4 kernel all the ciphers from the intel-aesni module get loaded whether I need them or not. As Jussi stated most people using distros probably won't need the serpent-avx-x86_64 module get loaded automatically, so it's probably better to leave it that way. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Tue, May 29, 2012 at 07:27:43PM -0700, Andi Kleen wrote: Also drivers should never print anything when they cannot find hardware. Remove that printk. I tried to be consistent with the existing ciphers in arch/x86/crypto. In serpent_sse2_glue.c and sha1_ssse3_glue.c it is done exactly that way, so if it will be decided to remove the printk in this patch it probably should also be removed in the other modules. - Johannes -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
I agree with that. Currently when I boot my PC with a new 3.4 kernel all the ciphers from the intel-aesni module get loaded whether I need them or not. As Jussi stated most people using distros probably won't need the serpent-avx-x86_64 module get loaded automatically, so it's probably better to leave it that way. That means you got a 50% chance to use the wrong serpent. This was a continuous problem with AESNI and the accelerated CRC, that is why the cpuid probing was implemented. Without some form of auto probing you may as well not bother with the optimization. -Andi -- a...@linux.intel.com -- Speaking for myself only. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Wed, May 30, 2012 at 01:36:51PM +0200, Johannes Goetzfried wrote: On Tue, May 29, 2012 at 07:27:43PM -0700, Andi Kleen wrote: Also drivers should never print anything when they cannot find hardware. Remove that printk. I tried to be consistent with the existing ciphers in arch/x86/crypto. In serpent_sse2_glue.c and sha1_ssse3_glue.c it is done exactly that way, so if it will be decided to remove the printk in this patch it probably should also be removed in the other modules. They are wrong and need to be fixed. Drivers can be loaded for all kinds of reasons, but they are not supposed to spam your kernel log when they do nothing. In fact I fixed all the existing ones with the cpuid changes, unfortunately somehow snuck in wrong code :-( -Andi -- a...@linux.intel.com -- Speaking for myself only. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Wed, May 30, 2012 at 06:26:45PM +0200, Andi Kleen wrote: I tried to be consistent with the existing ciphers in arch/x86/crypto. In serpent_sse2_glue.c and sha1_ssse3_glue.c it is done exactly that way, so if it will be decided to remove the printk in this patch it probably should also be removed in the other modules. They are wrong and need to be fixed. Drivers can be loaded for all kinds of reasons, but they are not supposed to spam your kernel log when they do nothing. In fact I fixed all the existing ones with the cpuid changes, unfortunately somehow snuck in wrong code :-( Hm, ok, removing the printk stuff is no big deal. I would fix this together with the cpuid probing, when I'm sure that this is the preferred new way :-) - Johannes -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Wed, May 30, 2012 at 05:39:49PM +0200, Andi Kleen wrote: I agree with that. Currently when I boot my PC with a new 3.4 kernel all the ciphers from the intel-aesni module get loaded whether I need them or not. As Jussi stated most people using distros probably won't need the serpent-avx-x86_64 module get loaded automatically, so it's probably better to leave it that way. That means you got a 50% chance to use the wrong serpent. You should always get the best one available. For example, when you request for aes all implementations of it will be loaded by modprobe. This was a continuous problem with AESNI and the accelerated CRC, that is why the cpuid probing was implemented. The only case where it doesn't work is if you have some variants built-in and the faster ones built as modules. But then the answer is to either build all of them as modules or all of them built-in so that priority-based selection can work. Can you provide an example where it doesn't work as intended? What we could do is to use the cpuid-based probing when an algorithm is needed to selectively load the relevant implementations instead of all of them. However, for most algorithms it won't make that big a difference since all the available ones will be loaded anyway. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
Can you provide an example where it doesn't work as intended? It works for the current algorithms except serpent. What we could do is to use the cpuid-based probing when an algorithm is needed to selectively load the relevant implementations instead of all of them. However, for most algorithms it won't make that big a difference since all the available ones will be loaded anyway. Yes cpuid probing does that. The only problem is just that serpent does not use cpuid probing. -Andi -- a...@linux.intel.com -- Speaking for myself only. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Wed, May 30, 2012 at 11:40:06PM +0200, Andi Kleen wrote: What we could do is to use the cpuid-based probing when an algorithm is needed to selectively load the relevant implementations instead of all of them. However, for most algorithms it won't make that big a difference since all the available ones will be loaded anyway. Yes cpuid probing does that. I was under the impression that using cpuid meant that the algorithm would be loaded regardless of whether it has been requested, as long as the cpuid matches. If that's not the case then there is certainly no reason to not use cpuid-based loading. Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Thu, May 31, 2012 at 07:44:48AM +1000, Herbert Xu wrote: On Wed, May 30, 2012 at 11:40:06PM +0200, Andi Kleen wrote: What we could do is to use the cpuid-based probing when an algorithm is needed to selectively load the relevant implementations instead of all of them. However, for most algorithms it won't make that big a difference since all the available ones will be loaded anyway. Yes cpuid probing does that. I was under the impression that using cpuid meant that the algorithm would be loaded regardless of whether it has been requested, as long as the cpuid matches. It loads it always correct. -Andi -- a...@linux.intel.com -- Speaking for myself only. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: serpent - add x86_64/avx assembler implementation
On Wed, May 30, 2012 at 11:55:37PM +0200, Andi Kleen wrote: On Thu, May 31, 2012 at 07:44:48AM +1000, Herbert Xu wrote: On Wed, May 30, 2012 at 11:40:06PM +0200, Andi Kleen wrote: What we could do is to use the cpuid-based probing when an algorithm is needed to selectively load the relevant implementations instead of all of them. However, for most algorithms it won't make that big a difference since all the available ones will be loaded anyway. Yes cpuid probing does that. I was under the impression that using cpuid meant that the algorithm would be loaded regardless of whether it has been requested, as long as the cpuid matches. It loads it always correct. You mean with cpuid it'll always load the algorithm if it is available even if noone is using it? Then it's probably not a good fit for serpent. Cheers, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: aesni-intel - fix unaligned cbc decrypt for x86-32
On Wed, May 30, 2012 at 01:43:08AM +0200, Mathias Krause wrote: The 32 bit variant of cbc(aes) decrypt is using instructions requiring 128 bit aligned memory locations but fails to ensure this constraint in the code. Fix this by loading the data into intermediate registers with load unaligned instructions. This fixes reported general protection faults related to aesni. References: https://bugzilla.kernel.org/show_bug.cgi?id=43223 Reported-by: Daniel gark...@mailueberfall.de Cc: sta...@kernel.org [v2.6.39+] Signed-off-by: Mathias Krause mini...@googlemail.com Have measured this against increasing alignmask to 15? Thanks, -- Email: Herbert Xu herb...@gondor.apana.org.au Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
