Re: [Patch V3] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
- Original Message - > On Wed, 2017-08-02 at 10:13 -0700, Megha Dey wrote: > > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is > > reading ahead beyond its intended data, and causing a crash if the next > > block is beyond page boundary: > > http://marc.info/?l=linux-crypto-vger=149373371023377 > > > > This patch makes sure that there is no overflow for any buffer length. > > > > It passes the tests written by Jan Stancek that revealed this problem: > > https://github.com/jstancek/sha1-avx2-crash > > Hi Jan, > > Is it ok to add your Tested-by? Yes, v3 patch is exactly the diff I was testing. Regards, Jan
Re: [Patch V2] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
On 08/02/2017 02:38 AM, Megha Dey wrote: > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is > reading ahead beyond its intended data, and causing a crash if the next > block is beyond page boundary: > http://marc.info/?l=linux-crypto-vger=149373371023377 > > This patch makes sure that there is no overflow for any buffer length. > > It passes the tests written by Jan Stancek that revealed this problem: > https://github.com/jstancek/sha1-avx2-crash > > Jan, can you verify this fix? Hi, Looks good, my tests (below) PASS-ed. I updated reproducer at [1] to try more than just single scenario. It now tries thousands of combinations with different starting offset within page and sizes of data chunks. (without patch) v4.13-rc3-102-g26c5ceb with avx2 enabled: [ 106.455175] sha_test module loaded [ 106.460458] data is at 0x8c88e58f8000, page_after_data: 0x8c88e58fa000 [ 106.468625] sha_test testing hash: sha1-generic, maxlen: 8192 [ 127.091237] sha_test hash: sha1-generic calculated 1764032 hashes [ 127.098038] sha_test testing hash: sha1-ni, maxlen: 8192 [ 127.108805] failed to alloc sha1-ni [ 127.112703] sha_test testing hash: sha1-avx, maxlen: 8192 [ 141.166611] sha_test hash: sha1-avx calculated 1764032 hashes [ 141.173023] sha_test testing hash: sha1-avx2, maxlen: 8192 [ 141.180046] BUG: unable to handle kernel paging request at 8c88e58fa000 [ 141.187817] IP: _begin+0x28/0x187 [ 141.191512] PGD 89c578067 [ 141.191513] P4D 89c578067 [ 141.194529] PUD c61f0a063 [ 141.197545] PMD c64cb1063 [ 141.200561] PTE 800c658fa062 [ 141.203577] [ 141.208832] Oops: [#1] SMP ... With patch "[Patch V2] crypto: x86/sha1 : Fix reads beyond the number of blocks passed": [ 406.323781] sha_test module loaded [ 406.329062] data is at 0x93ab97108000, page_after_data: 0x93ab9710a000 [ 406.337185] sha_test testing hash: sha1-generic, maxlen: 8192 [ 426.157242] sha_test hash: sha1-generic calculated 1764032 hashes [ 426.164043] sha_test testing hash: sha1-ni, maxlen: 8192 [ 426.174244] failed to alloc sha1-ni [ 426.178139] sha_test testing hash: sha1-avx, maxlen: 8192 [ 440.226240] sha_test hash: sha1-avx calculated 1764032 hashes [ 440.232651] sha_test testing hash: sha1-avx2, maxlen: 8192 [ 452.497988] sha_test hash: sha1-avx2 calculated 1764032 hashes [ 452.504495] sha_test testing hash: sha1-ssse3, maxlen: 8192 [ 467.319316] sha_test hash: sha1-ssse3 calculated 1764032 hashes [ 467.325922] sha_test done [ 471.827083] sha_test module unloaded I also ran a test at [2], which is comparing hashes produced by kernel with hashes produced by user-space utilities (e.g. 'sha1sum'). This test also PASS-ed. Regards, Jan [1] https://github.com/jstancek/sha1-avx2-crash [2] https://github.com/jstancek/kernel_sha_test/tree/sha1-avx2-4.13 > Herbert, can you re-enable sha1-avx2 once Jan has checked it out and > revert commit b82ce24426a4071da9529d726057e4e642948667 ? > > Originally-by: Ilya Albrekht <ilya.albre...@intel.com> > Signed-off-by: Megha Dey <megha@linux.intel.com> > Reported-by: Jan Stancek <jstan...@redhat.com>
Re: [bug] sha1-avx2 and read beyond
- Original Message - > On Wed, May 24, 2017 at 08:46:57AM -0400, Jan Stancek wrote: > > > > > > - Original Message - > > > Hi, > > > > > > I'm seeing rare crashes during NFS cthon with krb5 auth. After > > > some digging I arrived at potential problem with sha1-avx2. > > > > Adding more sha1_avx2 experts to CC. > > > > > > > > Problem appears to be that sha1_transform_avx2() reads beyond > > > number of blocks you pass, if it is an odd number. It appears > > > to try read one block more. This creates a problem if it falls > > > beyond a page and there's nothing there. > > > > As noted in my reply, worst case appears to be read ahead > > of up to 3 SHA1 blocks beyond end of data: > > http://marc.info/?l=linux-crypto-vger=149373371023377 > > > > +--+-+-+-+ > > | 2*SHA1_BLOCK_SIZE | 2*SHA1_BLOCK_SIZE | > > +--+-+-+-+ > > ^ page boundary > > ^ data end > > > > It is still reproducible with 4.12-rc2. > > Can someone from Intel please look into this? Otherwise we'll have > to disable sha-avx2. So I take it my workaround patch [1] is not acceptable in short-term as well? [1] http://marc.info/?l=linux-crypto-vger=149373371023377 Regards, Jan
Re: [bug] sha1-avx2 and read beyond
- Original Message - > Hi, > > I'm seeing rare crashes during NFS cthon with krb5 auth. After > some digging I arrived at potential problem with sha1-avx2. Adding more sha1_avx2 experts to CC. > > Problem appears to be that sha1_transform_avx2() reads beyond > number of blocks you pass, if it is an odd number. It appears > to try read one block more. This creates a problem if it falls > beyond a page and there's nothing there. As noted in my reply, worst case appears to be read ahead of up to 3 SHA1 blocks beyond end of data: http://marc.info/?l=linux-crypto-vger=149373371023377 +--+-+-+-+ | 2*SHA1_BLOCK_SIZE | 2*SHA1_BLOCK_SIZE | +--+-+-+-+ ^ page boundary ^ data end It is still reproducible with 4.12-rc2. Regards, Jan > > To demonstrate this, I made a module which computes some hashes > on module load. It allocates 3 pages, passes first two into > crypto_shash_update() and marks 3rd one as not present. > > When it runs for sha1-avx2, it runs into an Oops, trying to > access 3rd page: > > # git clone https://github.com/jstancek/sha1-avx2-crash.git > # cd sha1-avx2-crash/ > # make > # insmod sha1_test.ko > > [ 195.512669] sha1_test: loading out-of-tree module taints kernel. > [ 195.518716] sha1_test: module verification failed: signature and/or > required key missing - tainting kernel > [ 195.529754] sha_test module loaded > [ 195.533732] data is at 0x97e232ea8000, datalen: 12288, start_offset: > 3948, last_byte: 0x97e232ea9fff > [ 195.543529] page_after_data is at 0x97e232eaa000 > [ 195.548603] starting test for sha1-generic > [ 195.552703] count: 148 > [ 195.555073] starting test for sha1-ni > [ 195.561282] failed to alloc sha1-ni > [ 195.564776] starting test for sha1-avx > [ 195.568544] count: 148 > [ 195.570908] starting test for sha1-avx2 > [ 195.574751] count: 148 > [ 195.577135] BUG: unable to handle kernel paging request at > 97e232eaa000 > [ 195.584081] IP: _begin+0x173/0x187 > [ 195.587478] PGD 213e83067 > [ 195.587478] PUD 1033622063 > [ 195.590183] PMD 1033181063 > [ 195.592974] PTE 801032eaa062 > [ 195.595769] > [ 195.600487] Oops: [#1] SMP > [ 195.603627] Modules linked in: sha1_test(OE+) binfmt_misc intel_rapl > skx_edac edac_core x86_pkg_temp_thermal intel_powerclamp coretemp > kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel > vfat pcbc fat aesni_intel crypto_simd glue_helper cryptd ipmi_ss > if ipmi_si iTCO_wdt ioatdma mei_me ipmi_devintf iTCO_vendor_support pcspkr > joydev nfsd sg mei shpchp i2c_i801 dca lpc_ich wmi ipmi_msghand > ler nfs_acl lockd tpm_crb nfit auth_rpcgss libnvdimm grace acpi_pad > acpi_power_meter sunrpc ip_tables xfs libcrc32c sd_mod sr_mod cdrom as > t i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops > ttm drm i40e ahci ptp libahci crc32c_intel libata pps_core i2c > _core dm_mirror dm_region_hash dm_log dm_mod > [ 195.667322] CPU: 3 PID: 4725 Comm: insmod Tainted: G OE > 4.11.0-rc8 #1 > [ 195.674782] Hardware name: Intel Corporation S2600WFD/S2600WFD, BIOS > SE5C620.86B.01.00.0412.020920172159 02/09/2017 > [ 195.685185] task: 97e22a7b3b00 task.stack: a6f9a57e8000 > [ 195.691092] RIP: 0010:_begin+0x173/0x187 > [ 195.695005] RSP: 0018:a6f9a57eb5d8 EFLAGS: 00010202 > [ 195.700219] RAX: 24a63b1a RBX: de142126 RCX: > 455ad007 > [ 195.707336] RDX: 325cbadf RSI: 2c3b9293 RDI: > 9298ec68 > [ 195.714451] RBP: 45421007 R08: 9640a100 R09: > 97d3771be9d0 > [ 195.721567] R10: 97e232ea9f2c R11: 97e232eaa02c R12: > 531d8d12 > [ 195.728683] R13: 97e232ea9f6c R14: a6f9a57eb878 R15: > a6f9a57eb5d8 > [ 195.735799] FS: 7f675ac0c740() GS:97e23dac() > knlGS: > [ 195.743864] CS: 0010 DS: ES: CR0: 80050033 > [ 195.749596] CR2: 97e232eaa000 CR3: 0010394ec000 CR4: > 007406e0 > [ 195.756713] DR0: DR1: DR2: > > [ 195.763828] DR3: DR6: fffe0ff0 DR7: > 0400 > [ 195.770944] PKRU: 5554 > [ 195.773651] Call Trace: > [ 195.776105] ? dequeue_entity+0xed/0x400 > [ 195.780027] ? console_unlock+0x475/0x4a0 > [ 195.784027] ? sha1_base_init+0x40/0x40 > [ 195.787858] ? sha1_apply_transform_avx2+0x1a/0x30 > [ 195.792638] ? sha1_update+0xd3/0x130 > [ 195.796295] ? sha1_avx2_update+0x15/0x20 > [ 195.800301] ? crypto_shash_update+0x47/0x120 > [ 195.804650] ? calc_hash.constprop.0+0xdc/0xff [sha1_test] > [ 195.810122] ? sha1test_init+0x113/0x1000 [sha1_test] > [ 195.815163] ? 0xc02b7000 > [ 195.818473] ? do_one_initcall+0x51/0x1b0 > [ 195.822481] ? __vunmap+0x85/0xd0 > [ 195.825799] ? kmem_cache_alloc_trace+0x14b/0x1b0 > [ 195.830489] ? kfree+0x133/0x180 > [ 195.833716] ?
Re: [bug] sha1-avx2 and read beyond
On 04/30/2017 01:04 AM, Jan Stancek wrote:
> Hi,
>
> I'm seeing rare crashes during NFS cthon with krb5 auth. After
> some digging I arrived at potential problem with sha1-avx2.
>
> Problem appears to be that sha1_transform_avx2() reads beyond
> number of blocks you pass, if it is an odd number. It appears
> to try read one block more.
It's not just odd vs even number of blocks. It appears to be
doing read ahead (in size of 2 blocks). For example,
for data starting at page offset 1 with length 3967, it still
crashes on access to subsequent page.
Patch below fixes it for me, but it feels more like workaround.
Regards,
Jan
diff --git a/arch/x86/crypto/sha1_ssse3_glue.c
b/arch/x86/crypto/sha1_ssse3_glue.c
index fc61739150e7..736128267715 100644
--- a/arch/x86/crypto/sha1_ssse3_glue.c
+++ b/arch/x86/crypto/sha1_ssse3_glue.c
@@ -212,10 +212,41 @@ static bool avx2_usable(void)
static void sha1_apply_transform_avx2(u32 *digest, const char *data,
unsigned int rounds)
{
+ const char *last;
+ unsigned int rounds_avx2;
+
/* Select the optimal transform based on data block size */
- if (rounds >= SHA1_AVX2_BLOCK_OPTSIZE)
- sha1_transform_avx2(digest, data, rounds);
- else
+ if (rounds < SHA1_AVX2_BLOCK_OPTSIZE)
+ goto avx;
+
+ /*
+* sha1_transform_avx2() can read ahead couple blocks, which
+* can cause problems if it crosses page boundary and next
+* page doesn't exist. It operates on even number of blocks.
+* Code below checks for worst case, where it can access
+* up to 3 consecutive blocks after data end. In that case
+* sha1_transform_avx2() is passed 3 blocks less and rest
+* of data is handled by sha1_transform_avx().
+*
+* +--+-+-+-+
+* 2x SHA1_BLOCK_SIZE | 2*SHA1_BLOCK_SIZE
+* +--+-+-+-+
+*^ data end
+*/
+ last = data + (rounds + 3) * SHA1_BLOCK_SIZE - 1;
+ if (offset_in_page(last) >= 3 * SHA1_BLOCK_SIZE) {
+ rounds_avx2 = rounds;
+ } else {
+ rounds_avx2 = rounds - 3;
+ if (rounds_avx2 < SHA1_AVX2_BLOCK_OPTSIZE)
+ goto avx;
+ }
+
+ sha1_transform_avx2(digest, data, rounds_avx2);
+ data += SHA1_BLOCK_SIZE * rounds_avx2;
+ rounds -= rounds_avx2;
+avx:
+ if (rounds)
sha1_transform_avx(digest, data, rounds);
}
[bug] sha1-avx2 and read beyond
Hi, I'm seeing rare crashes during NFS cthon with krb5 auth. After some digging I arrived at potential problem with sha1-avx2. Problem appears to be that sha1_transform_avx2() reads beyond number of blocks you pass, if it is an odd number. It appears to try read one block more. This creates a problem if it falls beyond a page and there's nothing there. To demonstrate this, I made a module which computes some hashes on module load. It allocates 3 pages, passes first two into crypto_shash_update() and marks 3rd one as not present. When it runs for sha1-avx2, it runs into an Oops, trying to access 3rd page: # git clone https://github.com/jstancek/sha1-avx2-crash.git # cd sha1-avx2-crash/ # make # insmod sha1_test.ko [ 195.512669] sha1_test: loading out-of-tree module taints kernel. [ 195.518716] sha1_test: module verification failed: signature and/or required key missing - tainting kernel [ 195.529754] sha_test module loaded [ 195.533732] data is at 0x97e232ea8000, datalen: 12288, start_offset: 3948, last_byte: 0x97e232ea9fff [ 195.543529] page_after_data is at 0x97e232eaa000 [ 195.548603] starting test for sha1-generic [ 195.552703] count: 148 [ 195.555073] starting test for sha1-ni [ 195.561282] failed to alloc sha1-ni [ 195.564776] starting test for sha1-avx [ 195.568544] count: 148 [ 195.570908] starting test for sha1-avx2 [ 195.574751] count: 148 [ 195.577135] BUG: unable to handle kernel paging request at 97e232eaa000 [ 195.584081] IP: _begin+0x173/0x187 [ 195.587478] PGD 213e83067 [ 195.587478] PUD 1033622063 [ 195.590183] PMD 1033181063 [ 195.592974] PTE 801032eaa062 [ 195.595769] [ 195.600487] Oops: [#1] SMP [ 195.603627] Modules linked in: sha1_test(OE+) binfmt_misc intel_rapl skx_edac edac_core x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel vfat pcbc fat aesni_intel crypto_simd glue_helper cryptd ipmi_ss if ipmi_si iTCO_wdt ioatdma mei_me ipmi_devintf iTCO_vendor_support pcspkr joydev nfsd sg mei shpchp i2c_i801 dca lpc_ich wmi ipmi_msghand ler nfs_acl lockd tpm_crb nfit auth_rpcgss libnvdimm grace acpi_pad acpi_power_meter sunrpc ip_tables xfs libcrc32c sd_mod sr_mod cdrom as t i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm i40e ahci ptp libahci crc32c_intel libata pps_core i2c _core dm_mirror dm_region_hash dm_log dm_mod [ 195.667322] CPU: 3 PID: 4725 Comm: insmod Tainted: G OE 4.11.0-rc8 #1 [ 195.674782] Hardware name: Intel Corporation S2600WFD/S2600WFD, BIOS SE5C620.86B.01.00.0412.020920172159 02/09/2017 [ 195.685185] task: 97e22a7b3b00 task.stack: a6f9a57e8000 [ 195.691092] RIP: 0010:_begin+0x173/0x187 [ 195.695005] RSP: 0018:a6f9a57eb5d8 EFLAGS: 00010202 [ 195.700219] RAX: 24a63b1a RBX: de142126 RCX: 455ad007 [ 195.707336] RDX: 325cbadf RSI: 2c3b9293 RDI: 9298ec68 [ 195.714451] RBP: 45421007 R08: 9640a100 R09: 97d3771be9d0 [ 195.721567] R10: 97e232ea9f2c R11: 97e232eaa02c R12: 531d8d12 [ 195.728683] R13: 97e232ea9f6c R14: a6f9a57eb878 R15: a6f9a57eb5d8 [ 195.735799] FS: 7f675ac0c740() GS:97e23dac() knlGS: [ 195.743864] CS: 0010 DS: ES: CR0: 80050033 [ 195.749596] CR2: 97e232eaa000 CR3: 0010394ec000 CR4: 007406e0 [ 195.756713] DR0: DR1: DR2: [ 195.763828] DR3: DR6: fffe0ff0 DR7: 0400 [ 195.770944] PKRU: 5554 [ 195.773651] Call Trace: [ 195.776105] ? dequeue_entity+0xed/0x400 [ 195.780027] ? console_unlock+0x475/0x4a0 [ 195.784027] ? sha1_base_init+0x40/0x40 [ 195.787858] ? sha1_apply_transform_avx2+0x1a/0x30 [ 195.792638] ? sha1_update+0xd3/0x130 [ 195.796295] ? sha1_avx2_update+0x15/0x20 [ 195.800301] ? crypto_shash_update+0x47/0x120 [ 195.804650] ? calc_hash.constprop.0+0xdc/0xff [sha1_test] [ 195.810122] ? sha1test_init+0x113/0x1000 [sha1_test] [ 195.815163] ? 0xc02b7000 [ 195.818473] ? do_one_initcall+0x51/0x1b0 [ 195.822481] ? __vunmap+0x85/0xd0 [ 195.825799] ? kmem_cache_alloc_trace+0x14b/0x1b0 [ 195.830489] ? kfree+0x133/0x180 [ 195.833716] ? do_init_module+0x60/0x1fa [ 195.837638] ? load_module+0x162b/0x1b20 [ 195.841557] ? __symbol_put+0x60/0x60 [ 195.845217] ? ima_post_read_file+0x3d/0x80 [ 195.849397] ? security_kernel_post_read_file+0x6b/0x80 [ 195.854616] ? SYSC_finit_module+0xa6/0xf0 [ 195.858704] ? SyS_finit_module+0xe/0x10 [ 195.862622] ? do_syscall_64+0x67/0x180 [ 195.866450] ? entry_SYSCALL64_slow_path+0x25/0x25 [ 195.871230] Code: d0 02 c4 c1 7a 6f 82 90 00 00 00 21 c8 31 e8 42 8d 3c 27 41 03 77 44 c4 e2 40 f2 e9 8d 34 06 c4 63 7b f0 e7 1b c4 e3 7b f0 c7 02 c3 7d 18 85 90 00 00 00 01 21 d7 31 ef 42 8d 34 26 eb 00 41 [ 195.890035] RIP:
[PATCH] crypto: testmgr - add guard to dst buffer for ahash_export
Add a guard to 'state' buffer and warn if its consistency after
call to crypto_ahash_export() changes, so that any write that
goes beyond advertised statesize (and thus causing potential
memory corruption [1]) is more visible.
[1] https://marc.info/?l=linux-crypto-vger=147467656516085
Signed-off-by: Jan Stancek <jstan...@redhat.com>
Cc: Herbert Xu <herb...@gondor.apana.org.au>
Cc: Marcelo Cerri <marcelo.ce...@canonical.com>
---
crypto/testmgr.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 5c9d5a5e7b65..96343bcae01e 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -209,16 +209,19 @@ static int ahash_partial_update(struct ahash_request
**preq,
char *state;
struct ahash_request *req;
int statesize, ret = -EINVAL;
+ const char guard[] = { 0x00, 0xba, 0xad, 0x00 };
req = *preq;
statesize = crypto_ahash_statesize(
crypto_ahash_reqtfm(req));
- state = kmalloc(statesize, GFP_KERNEL);
+ state = kmalloc(statesize + sizeof(guard), GFP_KERNEL);
if (!state) {
pr_err("alt: hash: Failed to alloc state for %s\n", algo);
goto out_nostate;
}
+ memcpy(state + statesize, guard, sizeof(guard));
ret = crypto_ahash_export(req, state);
+ WARN_ON(memcmp(state + statesize, guard, sizeof(guard)));
if (ret) {
pr_err("alt: hash: Failed to export() for %s\n", algo);
goto out;
--
1.8.3.1
--
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: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7
- Original Message - > From: "Herbert Xu" <herb...@gondor.apana.org.au> > To: "Marcelo Cerri" <marcelo.ce...@canonical.com> > Cc: "Jan Stancek" <jstan...@redhat.com>, "rui y wang" <rui.y.w...@intel.com>, > mhce...@linux.vnet.ibm.com, > leosi...@linux.vnet.ibm.com, pfsmor...@linux.vnet.ibm.com, > linux-crypto@vger.kernel.org, > linuxppc-...@lists.ozlabs.org, linux-ker...@vger.kernel.org > Sent: Wednesday, 28 September, 2016 4:45:49 AM > Subject: Re: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7 > > On Tue, Sep 27, 2016 at 04:46:44PM -0300, Marcelo Cerri wrote: > > > > Can you check if the problem occurs with this patch? > > In light of the fact that padlock-sha is the correct example > to follow, you only need to add one line to the init_tfm fucntion > to update the descsize based on that of the fallback. Thanks for clearing up how this works in padlock-sha, but we are not exactly in same situation with p8_ghash. p8_ghash_init_tfm() already updates descsize. Problem in original report is that without custom export/import/statesize p8_ghash_alg.statesize gets initialized by shash_prepare_alg() to alg->descsize: crash> p p8_ghash_alg.statesize $1 = 56 testmgr allocates space for export based on crypto_shash_statesize(), but shash_default_export() writes based on crypto_shash_descsize(): [8.297902] state: c004b873aa80, statesize: 56 [8.297932] shash_default_export memcpy c004b873aa80 c004b8607da0, len: 76 so I think we need either: 1) make sure p8_ghash_alg.descsize is correct before we register shash, this is what Marcelo's last patch is doing 2) provide custom export/import/statesize for p8_ghash_alg Regards, Jan > > 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: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7
- Original Message -
> From: "Marcelo Cerri" <marcelo.ce...@canonical.com>
> To: "Jan Stancek" <jstan...@redhat.com>
> Cc: "rui y wang" <rui.y.w...@intel.com>, herb...@gondor.apana.org.au,
> mhce...@linux.vnet.ibm.com,
> leosi...@linux.vnet.ibm.com, pfsmor...@linux.vnet.ibm.com,
> linux-crypto@vger.kernel.org,
> linuxppc-...@lists.ozlabs.org, linux-ker...@vger.kernel.org
> Sent: Monday, 26 September, 2016 4:15:10 PM
> Subject: Re: [bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7
>
> Hi Jan,
>
> Just out of curiosity, have you tried to use "76" on both values to
> check if the problem still happens?
I did, I haven't seen any panics with such patch:
@@ -211,7 +212,7 @@ struct shash_alg p8_ghash_alg = {
.update = p8_ghash_update,
.final = p8_ghash_final,
.setkey = p8_ghash_setkey,
- .descsize = sizeof(struct p8_ghash_desc_ctx),
+ .descsize = sizeof(struct p8_ghash_desc_ctx) + 20,
.base = {
.cra_name = "ghash",
.cra_driver_name = "p8_ghash",
--
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
[bug] crypto/vmx/p8_ghash memory corruption in 4.8-rc7
Hi,
I'm chasing a memory corruption with 4.8-rc7 as I'm observing random Oopses
on ppc BE/LE systems (lpars, KVM guests). About 30% of issues is that
module list gets corrupted, and "cat /proc/modules" or "lsmod" triggers
an Oops, for example:
[ 88.486041] Unable to handle kernel paging request for data at address
0x0020
...
[ 88.487658] NIP [c020f820] m_show+0xa0/0x240
[ 88.487689] LR [c020f834] m_show+0xb4/0x240
[ 88.487719] Call Trace:
[ 88.487736] [c004b605bbb0] [c020f834] m_show+0xb4/0x240
(unreliable)
[ 88.487796] [c004b605bc50] [c045e73c] seq_read+0x36c/0x520
[ 88.487843] [c004b605bcf0] [c04e1014] proc_reg_read+0x84/0x120
[ 88.487889] [c004b605bd30] [c040df88] vfs_read+0xf8/0x380
[ 88.487934] [c004b605bde0] [c040fd40] SyS_read+0x60/0x110
[ 88.487981] [c004b605be30] [c0009590] system_call+0x38/0xec
0x20 offset is module_use->source, module_use is NULL because module.source_list
gets corrupted.
The source of corruption appears to originate from a 'ahash' test for p8_ghash:
cryptomgr_test
alg_test
alg_test_hash
test_hash
__test_hash
ahash_partial_update
shash_async_export
memcpy
With some extra traces [1], I'm seeing that ahash_partial_update() allocates 56
bytes
for 'state', and then crypto_ahash_export() writes 76 bytes into it:
[5.970887] __test_hash alg name p8_ghash, result: c4333ac0, key:
c004b860a500, req: c004b860a380
[5.970963] state: c4333f00, statesize: 56
[5.970995] shash_default_export memcpy c4333f00 c004b860a3e0,
len: 76
This seems to directly correspond with:
p8_ghash_alg.descsize = sizeof(struct p8_ghash_desc_ctx) == 56
shash_tfm->descsize = sizeof(struct p8_ghash_desc_ctx) +
crypto_shash_descsize(fallback) == 56 + 20
where 20 is presumably coming from "ghash_alg.descsize".
My gut feeling was that these 2 should match, but I'd love to hear
what crypto people think.
Thank you,
Jan
[1]
diff --git a/crypto/shash.c b/crypto/shash.c
index a051541..49fe182 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -188,6 +188,8 @@ EXPORT_SYMBOL_GPL(crypto_shash_digest);
static int shash_default_export(struct shash_desc *desc, void *out)
{
+ int len = crypto_shash_descsize(desc->tfm);
+ printk("shash_default_export memcpy %p %p, len: %d\n", out,
shash_desc_ctx(desc), len);
memcpy(out, shash_desc_ctx(desc), crypto_shash_descsize(desc->tfm));
return 0;
}
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 5c9d5a5..2e54579 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -218,6 +218,8 @@ static int ahash_partial_update(struct ahash_request **preq,
pr_err("alt: hash: Failed to alloc state for %s\n", algo);
goto out_nostate;
}
+ printk("state: %p, statesize: %d\n", state, statesize);
+
ret = crypto_ahash_export(req, state);
if (ret) {
pr_err("alt: hash: Failed to export() for %s\n", algo);
@@ -288,6 +290,7 @@ static int __test_hash(struct crypto_ahash *tfm, struct
hash_testvec *template,
"%s\n", algo);
goto out_noreq;
}
+ printk("__test_hash alg name %s, result: %p, key: %p, req: %p\n", algo,
result, key, req);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
tcrypt_complete, );
--
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
[PATCH] crypto: qat - make qat_asym_algs.o depend on asn1 headers
Parallel build can sporadically fail because asn1 headers may not be built yet by the time qat_asym_algs.o is compiled: drivers/crypto/qat/qat_common/qat_asym_algs.c:55:32: fatal error: qat_rsapubkey-asn1.h: No such file or directory #include "qat_rsapubkey-asn1.h" Signed-off-by: Jan Stancek <jstan...@redhat.com> Cc: Tadeusz Struk <tadeusz.st...@intel.com> Cc: Herbert Xu <herb...@gondor.apana.org.au> --- drivers/crypto/qat/qat_common/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/qat/qat_common/Makefile b/drivers/crypto/qat/qat_common/Makefile index 6d74b91f2152..5fc3dbb9ada0 100644 --- a/drivers/crypto/qat/qat_common/Makefile +++ b/drivers/crypto/qat/qat_common/Makefile @@ -2,6 +2,7 @@ $(obj)/qat_rsapubkey-asn1.o: $(obj)/qat_rsapubkey-asn1.c \ $(obj)/qat_rsapubkey-asn1.h $(obj)/qat_rsaprivkey-asn1.o: $(obj)/qat_rsaprivkey-asn1.c \ $(obj)/qat_rsaprivkey-asn1.h +$(obj)/qat_asym_algs.o: $(obj)/qat_rsapubkey-asn1.h $(obj)/qat_rsaprivkey-asn1.h clean-files += qat_rsapubkey-asn1.c qat_rsapubkey-asn1.h clean-files += qat_rsaprivkey-asn1.c qat_rsaprivkey-asn1.h -- 1.8.3.1 -- 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
[PATCH] crypto: nx-sha256/512: respect sg limit bounds when building sg lists
Commit 000851119e80 changed sha256/512 update functions to
pass more data to nx_build_sg_list(), which ends with
sg list overflows and usually with update functions failing
for data larger than max_sg_len * NX_PAGE_SIZE.
This happens because:
- both total and to_process are updated, which leads to
to_process getting overflowed for some data lengths
For example:
In first iteration total is 50, and let's assume to_process
is 30 due to sg limits. At the end of first iteration total is
set to 20. At start of 2nd iteration to_process overflows on:
to_process = total - to_process;
- in_sg is not reset to nx_ctx-in_sg after each iteration
- nx_build_sg_list() is hitting overflow because the amount of data
passed to it would require more than sgmax elements
- as consequence of previous item, data stored in overflowed sg list
may no longer be aligned to SHA*_BLOCK_SIZE
This patch changes sha256/512 update functions so that to_process
respects sg limits and never tries to pass more data to
nx_build_sg_list() to avoid overflows. to_process is calculated
as minimum of total and sg limits at start of every iteration.
Fixes: 000851119e80 (crypto: nx - Fix SHA concurrence issue and sg
limit bounds)
Signed-off-by: Jan Stancek jstan...@redhat.com
Cc: Leonidas Da Silva Barbosa leosi...@linux.vnet.ibm.com
Cc: Marcelo Henrique Cerri mhce...@linux.vnet.ibm.com
Cc: Fionnuala Gunter f...@linux.vnet.ibm.com
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
---
drivers/crypto/nx/nx-sha256.c | 27 ---
drivers/crypto/nx/nx-sha512.c | 28
2 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index 08f8d5cd6334..becb738c897b 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -71,7 +71,6 @@ static int nx_sha256_update(struct shash_desc *desc, const u8
*data,
struct sha256_state *sctx = shash_desc_ctx(desc);
struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(desc-tfm-base);
struct nx_csbcpb *csbcpb = (struct nx_csbcpb *)nx_ctx-csbcpb;
- struct nx_sg *in_sg;
struct nx_sg *out_sg;
u64 to_process = 0, leftover, total;
unsigned long irq_flags;
@@ -97,7 +96,6 @@ static int nx_sha256_update(struct shash_desc *desc, const u8
*data,
NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
- in_sg = nx_ctx-in_sg;
max_sg_len = min_t(u64, nx_ctx-ap-sglen,
nx_driver.of.max_sg_len/sizeof(struct nx_sg));
max_sg_len = min_t(u64, max_sg_len,
@@ -114,17 +112,12 @@ static int nx_sha256_update(struct shash_desc *desc,
const u8 *data,
}
do {
- /*
-* to_process: the SHA256_BLOCK_SIZE data chunk to process in
-* this update. This value is also restricted by the sg list
-* limits.
-*/
- to_process = total - to_process;
- to_process = to_process ~(SHA256_BLOCK_SIZE - 1);
+ int used_sgs = 0;
+ struct nx_sg *in_sg = nx_ctx-in_sg;
if (buf_len) {
data_len = buf_len;
- in_sg = nx_build_sg_list(nx_ctx-in_sg,
+ in_sg = nx_build_sg_list(in_sg,
(u8 *) sctx-buf,
data_len,
max_sg_len);
@@ -133,15 +126,27 @@ static int nx_sha256_update(struct shash_desc *desc,
const u8 *data,
rc = -EINVAL;
goto out;
}
+ used_sgs = in_sg - nx_ctx-in_sg;
}
+ /* to_process: SHA256_BLOCK_SIZE aligned chunk to be
+* processed in this iteration. This value is restricted
+* by sg list limits and number of sgs we already used
+* for leftover data. (see above)
+* In ideal case, we could allow NX_PAGE_SIZE * max_sg_len,
+* but because data may not be aligned, we need to account
+* for that too. */
+ to_process = min_t(u64, total,
+ (max_sg_len - 1 - used_sgs) * NX_PAGE_SIZE);
+ to_process = to_process ~(SHA256_BLOCK_SIZE - 1);
+
data_len = to_process - buf_len;
in_sg = nx_build_sg_list(in_sg, (u8 *) data,
data_len, max_sg_len);
nx_ctx-op.inlen = (nx_ctx-in_sg - in_sg) * sizeof(struct
nx_sg);
- to_process = (data_len + buf_len);
+ to_process = data_len + buf_len;
leftover = total - to_process;
/*
diff
