Re: [PATCH] crypto: fix handling of ccm vectors with null assoc data

2009-01-21 Thread Neil Horman
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

2009-01-21 Thread Herbert Xu
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

2009-01-21 Thread Huang Ying
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

2009-01-21 Thread Herbert Xu
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