* [PATCH] crypto: fix handling of ccm vectors with null assoc data
@ 2009-01-21 21:41 Jarod Wilson
2009-01-22 1:07 ` Neil Horman
0 siblings, 1 reply; 3+ messages in thread
From: Jarod Wilson @ 2009-01-21 21:41 UTC (permalink / raw)
To: linux-crypto; +Cc: linux-kernel, Herbert Xu, Neil Horman
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 ffff810064ddaec0 RIP:
[<ffffffff8864c4d7>] :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:[<ffffffff8864c4d7>] [<ffffffff8864c4d7>] :ccm:get_data_to_compute+0x1a6/0x1d6
RSP: 0018:ffff8100134434e8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff8100104898b0 RCX: ffffffffab6aea10
RDX: 0000000000000010 RSI: ffff8100104898c0 RDI: ffff810064ddaec0
RBP: 0000000000000000 R08: ffff8100104898b0 R09: 0000000000000000
R10: ffff8100103bac84 R11: ffff8100104898b0 R12: ffff810010489858
R13: ffff8100104898b0 R14: ffff8100103bac00 R15: 0000000000000000
FS: 00002ab881adfd30(0000) GS:ffffffff803ac000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffff810064ddaec0 CR3: 0000000012a88000 CR4: 00000000000006e0
Process crypto-tester (pid: 12844, threadinfo ffff810013442000, task ffff81003d165860)
Stack: ffff8100103bac00 ffff8100104898e8 ffff8100134436f8 ffffffff00000000
0000000000000000 ffff8100104898b0 0000000000000000 ffff810010489858
0000000000000000 ffff8100103bac00 ffff8100134436f8 ffffffff8864c634
Call Trace:
[<ffffffff8864c634>] :ccm:crypto_ccm_auth+0x12d/0x140
[<ffffffff8864cf73>] :ccm:crypto_ccm_decrypt+0x161/0x23a
[<ffffffff88633643>] :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 <jarod@redhat.com>
---
crypto/ccm.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/crypto/ccm.c b/crypto/ccm.c
index 7cf7e5a..c36d654 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -266,6 +266,8 @@ static int crypto_ccm_auth(struct aead_request *req, struct scatterlist *plain,
if (assoclen) {
pctx->ilen = format_adata(idata, assoclen);
get_data_to_compute(cipher, pctx, req->assoc, req->assoclen);
+ } else {
+ pctx->ilen = 0;
}
/* compute plaintext into mac */
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] crypto: fix handling of ccm vectors with null assoc data
2009-01-21 21:41 [PATCH] crypto: fix handling of ccm vectors with null assoc data Jarod Wilson
@ 2009-01-22 1:07 ` Neil Horman
2009-01-22 8:58 ` Herbert Xu
0 siblings, 1 reply; 3+ messages in thread
From: Neil Horman @ 2009-01-22 1:07 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-crypto, linux-kernel, Herbert Xu
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 ffff810064ddaec0 RIP:
> [<ffffffff8864c4d7>] :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:[<ffffffff8864c4d7>] [<ffffffff8864c4d7>] :ccm:get_data_to_compute+0x1a6/0x1d6
> RSP: 0018:ffff8100134434e8 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff8100104898b0 RCX: ffffffffab6aea10
> RDX: 0000000000000010 RSI: ffff8100104898c0 RDI: ffff810064ddaec0
> RBP: 0000000000000000 R08: ffff8100104898b0 R09: 0000000000000000
> R10: ffff8100103bac84 R11: ffff8100104898b0 R12: ffff810010489858
> R13: ffff8100104898b0 R14: ffff8100103bac00 R15: 0000000000000000
> FS: 00002ab881adfd30(0000) GS:ffffffff803ac000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: ffff810064ddaec0 CR3: 0000000012a88000 CR4: 00000000000006e0
> Process crypto-tester (pid: 12844, threadinfo ffff810013442000, task ffff81003d165860)
> Stack: ffff8100103bac00 ffff8100104898e8 ffff8100134436f8 ffffffff00000000
> 0000000000000000 ffff8100104898b0 0000000000000000 ffff810010489858
> 0000000000000000 ffff8100103bac00 ffff8100134436f8 ffffffff8864c634
> Call Trace:
> [<ffffffff8864c634>] :ccm:crypto_ccm_auth+0x12d/0x140
> [<ffffffff8864cf73>] :ccm:crypto_ccm_decrypt+0x161/0x23a
> [<ffffffff88633643>] :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 <jarod@redhat.com>
>
This looks good to me. Thanks Jarod!
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-01-22 8:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-21 21:41 [PATCH] crypto: fix handling of ccm vectors with null assoc data Jarod Wilson
2009-01-22 1:07 ` Neil Horman
2009-01-22 8:58 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox