| Message ID | 20151002140014.GI18263@oracle.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Herbert Xu |
| Headers | show |
On Fri, Oct 02, 2015 at 02:00:14PM +0000, Sowmini Varadhan wrote: > > I'm getting a lot of unaligned access messages each time I > do "modprobe [-r] <module>" on sparc: > > Kernel unaligned access at TPC[6ad9b4] pkcs7_verify+0x1ec/0x5e0 > Kernel unaligned access at TPC[6a5484] crypto_shash_finup+0xc/0x5c > Kernel unaligned access at TPC[6a5390] crypto_shash_update+0xc/0x54 > Kernel unaligned access at TPC[10150308] sha1_sparc64_update+0x14/0x5c [sha1_sparc64] > Kernel unaligned access at TPC[101501ac] __sha1_sparc64_update+0xc/0x98 [sha1_sparc64] > > Looks like these are being caused by an unaligned desc at > > desc = digest + digest_size; > > Doing this: > > --- a/crypto/asymmetric_keys/pkcs7_verify.c > +++ b/crypto/asymmetric_keys/pkcs7_verify.c > @@ -46,7 +46,8 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7, > return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm); > > desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); > - sinfo->sig.digest_size = digest_size = crypto_shash_digestsize(tfm); > + sinfo->sig.digest_size = digest_size = > + ALIGN(crypto_shash_digestsize(tfm), sizeof (*desc)); > > ret = -ENOMEM; > digest = kzalloc(digest_size + desc_size, GFP_KERNEL); > > makes the unaliagned message go away, but I dont know if > sinfo->sig.digest_size needs to be set to the (unaligned) raw > value of crypto_shash_digestsize() itself, and how to verify that > this doesnt break something else in crypto What hash algorithm were you using? Thanks,
On (10/08/15 21:15), Herbert Xu wrote: > > desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); > > - sinfo->sig.digest_size = digest_size = crypto_shash_digestsize(tfm); > > + sinfo->sig.digest_size = digest_size = > > + ALIGN(crypto_shash_digestsize(tfm), sizeof (*desc)); : > What hash algorithm were you using? Algorithm is sha1. From printk, crypto_shash_descsize(tfm) comes out to 0x60, digest_size to 0x14. Stack trace (for each modprobe [-r]) is pkcs7_verify+0x1d0/0x5e0 system_verify_data+0x54/0xb4 mod_verify_sig+0xa0/0xc4 load_module+0x48/0x16a0 SyS_init_module+0x114/0x128 linux_sparc_syscall+0x34/0x44 --Sowmini -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 08, 2015 at 10:43:43AM -0400, Sowmini Varadhan wrote: > On (10/08/15 21:15), Herbert Xu wrote: > > > desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); > > > - sinfo->sig.digest_size = digest_size = crypto_shash_digestsize(tfm); > > > + sinfo->sig.digest_size = digest_size = > > > + ALIGN(crypto_shash_digestsize(tfm), sizeof (*desc)); > : > > What hash algorithm were you using? > > Algorithm is sha1. From printk, crypto_shash_descsize(tfm) comes out > to 0x60, digest_size to 0x14. Stack trace (for each modprobe [-r]) is > > pkcs7_verify+0x1d0/0x5e0 > system_verify_data+0x54/0xb4 > mod_verify_sig+0xa0/0xc4 > load_module+0x48/0x16a0 > SyS_init_module+0x114/0x128 > linux_sparc_syscall+0x34/0x44 Thanks. We have two bugs here. First of all pkcs7_verify definitely shouldn't place the structure after the digest without aligning the pointer. So something like your patch is needed (but please use alignof instead of sizeof). Also don't put in digest_size but instead align the pointer like desc = PTR_ALIGN(digest + digest_size, ...) The sparc sha algorithms themselves need to declare the alignment that they require. Currently they claim to be able to handle any alignment which appears to not be the case. Cheers,
On (10/12/15 21:32), Herbert Xu wrote: > Thanks. We have two bugs here. First of all pkcs7_verify definitely > shouldn't place the structure after the digest without aligning the > pointer. So something like your patch is needed (but please use > alignof instead of sizeof). Also don't put in digest_size but > instead align the pointer like > > desc = PTR_ALIGN(digest + digest_size, ...) That patch might not be rock-solid by itself though. I was seeing some panics/crashes when I was running with that patch, so I backed it off temporarily - should sinfo->sig.digest_size be set to the aligned value? > The sparc sha algorithms themselves need to declare the alignment > that they require. Currently they claim to be able to handle any > alignment which appears to not be the case. How would one do that correctly? I'm not a crypto expert, but I can help test the patch.. --Sowmini -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Mon, 12 Oct 2015 21:32:09 +0800 > The sparc sha algorithms themselves need to declare the alignment > that they require. Currently they claim to be able to handle any > alignment which appears to not be the case. The sparc SHA assembler can handle arbitrary alignment. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 12, 2015 at 07:06:34AM -0700, David Miller wrote: > From: Herbert Xu <herbert@gondor.apana.org.au> > Date: Mon, 12 Oct 2015 21:32:09 +0800 > > > The sparc sha algorithms themselves need to declare the alignment > > that they require. Currently they claim to be able to handle any > > alignment which appears to not be the case. > > The sparc SHA assembler can handle arbitrary alignment. Right. The warnings that originated in sha1_sparc64 are probably just the result of an unaligned desc structure caused by the bug in pkcs7_verify. Thanks for the confirmation.
--- a/crypto/asymmetric_keys/pkcs7_verify.c +++ b/crypto/asymmetric_keys/pkcs7_verify.c @@ -46,7 +46,8 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7, return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm); desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); - sinfo->sig.digest_size = digest_size = crypto_shash_digestsize(tfm); + sinfo->sig.digest_size = digest_size = + ALIGN(crypto_shash_digestsize(tfm), sizeof (*desc)); ret = -ENOMEM; digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
Hi, I'm getting a lot of unaligned access messages each time I do "modprobe [-r] <module>" on sparc: Kernel unaligned access at TPC[6ad9b4] pkcs7_verify+0x1ec/0x5e0 Kernel unaligned access at TPC[6a5484] crypto_shash_finup+0xc/0x5c Kernel unaligned access at TPC[6a5390] crypto_shash_update+0xc/0x54 Kernel unaligned access at TPC[10150308] sha1_sparc64_update+0x14/0x5c [sha1_sparc64] Kernel unaligned access at TPC[101501ac] __sha1_sparc64_update+0xc/0x98 [sha1_sparc64] Looks like these are being caused by an unaligned desc at desc = digest + digest_size; Doing this: makes the unaliagned message go away, but I dont know if sinfo->sig.digest_size needs to be set to the (unaligned) raw value of crypto_shash_digestsize() itself, and how to verify that this doesnt break something else in crypto --Sowmini -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html