Re: [PATCH] crypto: fix handling of ccm vectors with null assoc data
On Wed, Jan 21, 2009 at 04:41:01PM -0500, Jarod Wilson wrote: Its a valid use case to have null associated data in a ccm vector, but this case isn't being handled properly right now. The following ccm decryption/verification test vector, using the rfc4309 implementation regularly triggers a panic, as will any other vector with null assoc data: * key: ab2f8a74b71cd2b1ff802e487d82f8b9 * iv: c6fb7d800d13abd8a6b2d8 * Associated Data: [NULL] * Tag Length: 8 * input: d5e8939fc7892e2b The resulting panic looks like so: Unable to handle kernel paging request at 810064ddaec0 RIP: [8864c4d7] :ccm:get_data_to_compute+0x1a6/0x1d6 PGD 8063 PUD 0 Oops: 0002 [1] SMP last sysfs file: /module/libata/version CPU 0 Modules linked in: crypto_tester_kmod(U) seqiv krng ansi_cprng chainiv rng ctr aes_generic aes_x86_64 ccm cryptomgr testmgr_cipher testmgr aead crypto_blkcipher crypto_a lgapi des ipv6 xfrm_nalgo crypto_api autofs4 hidp l2cap bluetooth nfs lockd fscache nfs_acl sunrpc ip_conntrack_netbios_ns ipt_REJECT xt_state ip_conntrack nfnetlink xt_ tcpudp iptable_filter ip_tables x_tables dm_mirror dm_log dm_multipath scsi_dh dm_mod video hwmon backlight sbs i2c_ec button battery asus_acpi acpi_memhotplug ac lp sg snd_intel8x0 snd_ac97_codec ac97_bus snd_seq_dummy snd_seq_oss joydev snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_mixer_oss ide_cd snd_pcm floppy parport_p c shpchp e752x_edac snd_timer e1000 i2c_i801 edac_mc snd soundcore snd_page_alloc i2c_core cdrom parport serio_raw pcspkr ata_piix libata sd_mod scsi_mod ext3 jbd uhci_h cd ohci_hcd ehci_hcd Pid: 12844, comm: crypto-tester Tainted: G 2.6.18-128.el5.fips1 #1 RIP: 0010:[8864c4d7] [8864c4d7] :ccm:get_data_to_compute+0x1a6/0x1d6 RSP: 0018:8100134434e8 EFLAGS: 00010246 RAX: RBX: 8100104898b0 RCX: ab6aea10 RDX: 0010 RSI: 8100104898c0 RDI: 810064ddaec0 RBP: R08: 8100104898b0 R09: R10: 8100103bac84 R11: 8100104898b0 R12: 810010489858 R13: 8100104898b0 R14: 8100103bac00 R15: FS: 2ab881adfd30() GS:803ac000() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: 810064ddaec0 CR3: 12a88000 CR4: 06e0 Process crypto-tester (pid: 12844, threadinfo 810013442000, task 81003d165860) Stack: 8100103bac00 8100104898e8 8100134436f8 8100104898b0 810010489858 8100103bac00 8100134436f8 8864c634 Call Trace: [8864c634] :ccm:crypto_ccm_auth+0x12d/0x140 [8864cf73] :ccm:crypto_ccm_decrypt+0x161/0x23a [88633643] :crypto_tester_kmod:cavs_test_rfc4309_ccm+0x4a5/0x559 [...] The above is from a RHEL5-based kernel, but upstream is susceptible too. The fix is trivial: in crypto/ccm.c:crypto_ccm_auth(), pctx-ilen contains whatever was in memory when pctx was allocated if assoclen is 0. The tested fix is to simply add an else clause setting pctx-ilen to 0 for the assoclen == 0 case, so that get_data_to_compute() doesn't try doing things its not supposed to. Signed-off-by: Jarod Wilson ja...@redhat.com This looks good to me. Thanks Jarod! Acked-by: Neil Horman nhor...@tuxdriver.com -- 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: [RFC] per-CPU cryptd thread implementation based on workqueue
On Thu, Jan 22, 2009 at 10:32:17AM +0800, Huang Ying wrote: This is the first attempt to use a dedicate workqueue for crypto. It is not intended to be merged. Please feedback your comments, especially on desgin. Thanks for the patch! + spin_lock_init(cpu_queue-lock); Since we're switching to per-cpu queues it would be good to just kill the spin lock. AFAICS the only place you really need it is in cryptd_tfm_in_queue. That's just used for debugging so we can just kill it and lose this spin lock. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmVHI~} 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: [RFC] per-CPU cryptd thread implementation based on workqueue
On Thu, 2009-01-22 at 11:04 +0800, Herbert Xu wrote: On Thu, Jan 22, 2009 at 10:32:17AM +0800, Huang Ying wrote: This is the first attempt to use a dedicate workqueue for crypto. It is not intended to be merged. Please feedback your comments, especially on desgin. Thanks for the patch! + spin_lock_init(cpu_queue-lock); Since we're switching to per-cpu queues it would be good to just kill the spin lock. AFAICS the only place you really need it is in cryptd_tfm_in_queue. That's just used for debugging so we can just kill it and lose this spin lock. Yes. Except that, now we do not need a spin lock really. I think the spin lock may be useful if we enqueue a request on other CPU's queue to do load balance. And if it is possible that the work_struct to be executed on CPU other original CPU for CPU hotplug (current code do not). Best Regards, Huang Ying signature.asc Description: This is a digitally signed message part
Re: [RFC] per-CPU cryptd thread implementation based on workqueue
On Thu, Jan 22, 2009 at 03:15:58PM +0800, Huang Ying wrote: Yes. Except that, now we do not need a spin lock really. I think the spin lock may be useful if we enqueue a request on other CPU's queue to do load balance. And if it is possible that the work_struct to be executed on CPU other original CPU for CPU hotplug (current code do not). Right, but I think load-balancing should be explicitly enabled, i.e., we probably don't want to do it by default for AES-NI. The way I see load balancing work is if you had a template that sat on top of cryptd pass the requests to the cryptd on a CPU it chooses. Then we can enable it for any algorithm in the system simply by instantiating that template for it. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmVHI~} 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