Re: [Patch V2] crypto: x86/sha1 : Fix reads beyond the number of blocks passed

[Tim Chen]

On 08/02/2017 03:39 AM, Jan Stancek wrote:
> 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.

Great.  Should the fix be picked up also in the stable branches since
it was disabled in the stable releases?  Or the changes are too
much for stable?

Tim

Re: [Patch V2] crypto: x86/sha1 : Fix reads beyond the number of blocks passed

[Jan Stancek]

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: [Patch V2] crypto: x86/sha1 : Fix reads beyond the number of blocks passed

[Herbert Xu]

On Tue, Aug 01, 2017 at 05:38:32PM -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
> 
> Jan, can you verify this fix?
> Herbert, can you re-enable sha1-avx2 once Jan has checked it out and
> revert commit b82ce24426a4071da9529d726057e4e642948667 ?

Can you please include the hunk to actually reenable sha1-avx2
in your patch? Thanks!
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt