Bug #4320
closedBus Error in digest/sha2 on sparc
Description
=begin
Most likely this is caused due to misaligned memory. Any comment is greatly appreciated.
This bug can reproduce at Ruby 1.8, too.
ruby -e "require 'digest/sha2'; d= Digest::SHA256.new; ['a' * 97, 'a' * 97].each {|i| d.update(i)}; p d"
-e:1: [BUG] Bus Error
ruby 1.9.2p136 (2010-12-25 revision 30365) [sparc-solaris2.10]
-- control frame ----------
c:0007 p:---- s:0019 b:0019 l:000018 d:000018 CFUNC :update
c:0006 p:0014 s:0015 b:0015 l:0015ac d:000014 BLOCK -e:1
c:0005 p:---- s:0012 b:0012 l:000011 d:000011 FINISH
c:0004 p:---- s:0010 b:0010 l:000009 d:000009 CFUNC :each
c:0003 p:0054 s:0007 b:0007 l:0015ac d:000ed0 EVAL -e:1
c:0002 p:---- s:0004 b:0004 l:000003 d:000003 FINISH
c:0001 p:0000 s:0002 b:0002 l:0015ac d:0015ac TOP
=end
Files
Updated by usa (Usaku NAKAMURA) almost 14 years ago
- Status changed from Open to Assigned
- Assignee set to knu (Akinori MUSHA)
=begin
=end
Updated by slink (Nils Goroll) over 13 years ago
=begin
This indeed is an alignment issue, SHA256_Update calls SHA256_Transform with possibly unaligned data, but the latter needs its data argument be aligned on platforms which do not support unaligned word access. The same bug exists for SHA384 and SHA512
I have a fix ready, currently testing it.
here's a failing test case for SHA512, which will also hit for SHA384 (which uses the same code).
ruby -e "require 'digest/sha2'; d= Digest::SHA512.new; ['a' * 57, 'b' * 199].each {|i| d.update(i)}; p d"
-e:1: [BUG] Bus Error
ruby 1.9.2p136 (2010-12-25 revision 30365) [sparc-solaris2.10]
-- control frame ----------
c:0007 p:---- s:0019 b:0019 l:000018 d:000018 CFUNC :update
c:0006 p:0014 s:0015 b:0015 l:001504 d:000014 BLOCK -e:1
c:0005 p:---- s:0012 b:0012 l:000011 d:000011 FINISH
c:0004 p:---- s:0010 b:0010 l:000009 d:000009 CFUNC :each
c:0003 p:0054 s:0007 b:0007 l:001504 d:000db0 EVAL -e:1
c:0002 p:---- s:0004 b:0004 l:000003 d:000003 FINISH
c:0001 p:0000 s:0002 b:0002 l:001504 d:001504 TOP
-- Ruby level backtrace information ----------------------------------------
-e:1:in <main>' -e:1:in
each'
-e:1:in block in <main>' -e:1:in
update'
[NOTE]
You may have encountered a bug in the Ruby interpreter or extension libraries.
Bug reports are welcome.
For details: http://www.ruby-lang.org/bugreport.html
Abort (core dumped)
=end
Updated by slink (Nils Goroll) over 13 years ago
- File ruby_issue_4320__digest_sha2_alignment.patch ruby_issue_4320__digest_sha2_alignment.patch added
=begin
I'll attach a proposed fix:
- make ((|context->buffer|)) an array of the type being expected by (({SHAXXX_Transform})) (rather than a byte array), so our compiler will align it, if necessary
- remove now unneeded casts when passing the buffer to (({SHAXXX_Transform}))
- use a cast-to-uint8 version of the buffer for byte access
the actual fix:
- for platforms which are not known to accept unaligned access to words (conditions taken from ((%regint.h%))), use existing buffering code in (({SHAXXX_Update})) to align data by copying
I am not too happy about the (({PLAT_NEED_ALIGNED_WORD_ACCESS})), it appears to me that checking architecture alignment requirements should be done in ((%autoconf%)) or similar. Also, the simplistic (({ALIGNOF()})) macro will not return the minimal alignment requirement, but rather the alignment the compiler has chosen for a struct (which may be more than minimal).
At any rate, the patch does the job.
=== Test
I've done basic regression testing by
- comparing the output of the following commands between x86 with unpatched 1.8.7-p175 i386-solaris2.11 and patched 1.9.2-p180:
ruby -e "require 'digest/sha2'; 1.upto(290+20) { |i| 1.upto(290+20) { |j| d= Digest::SHA256.new; p i.to_s+' '+j.to_s; ['a' * i, 'b' * j].each {|c| d.update(c)}; p d}}" >/tmp/sha_256.out &
ruby -e "require 'digest/sha2'; 1.upto(290+20) { |i| 1.upto(290+20) { |j| d= Digest::SHA512.new; p i.to_s+' '+j.to_s; ['a' * i, 'b' * j].each {|c| d.update(c)}; p d}}" >/tmp/sha_512.out &
- checksumming the contents of Solaris 10 SPARC ((%/usr/bin%)) with the vesions given above usind:
find . -type f | ruby -e "require 'digest/sha2'; ARGF.each_line { |fname| fname = fname.chomp; begin; p fname+': '+Digest::SHA256.file(fname).hexdigest; rescue; end; } " >/tmp/ruby_hash_n
So I presume SHA2 is working OK.
((%make check%)) returns
8063 tests, 1870484 assertions, 21 failures, 35 errors, 2 skips
but none of the failures/errors is related to digest.
=end
Updated by nahi (Hiroshi Nakamura) over 13 years ago
- Assignee changed from knu (Akinori MUSHA) to nahi (Hiroshi Nakamura)
- Target version changed from 1.9.2 to 1.9.3
Updated by nahi (Hiroshi Nakamura) over 13 years ago
- File sha2.c.diff sha2.c.diff added
Nils, thanks for the patch, and sorry for late reply.
Since sha2.c has an upstream version http://www.aarongifford.com/computers/sha.html (which does not support sparc-solaris I think), I want to minimize the patch. Here's my version based on yours. Can you try it? It passes 'make test-all TESTS=digest' on my SunOS 5.8 on sparc.
Updated by nahi (Hiroshi Nakamura) over 13 years ago
This looks much simpler. I'll check this patch tomorrow.
Index: ext/digest/sha2/sha2.c¶
--- ext/digest/sha2/sha2.c (revision 32536)
+++ ext/digest/sha2/sha2.c (working copy)
@@ -559,7 +559,8 @@
}
while (len >= SHA256_BLOCK_LENGTH) {
/* Process as many complete blocks as we can */
-
SHA256_Transform(context, (sha2_word32*)data);
-
MEMCPY_BCOPY(context->buffer, data, SHA256_BLOCK_LENGTH);
-
SHA256_Transform(context, (sha2_word32*)context->buffer); context->bitcount += SHA256_BLOCK_LENGTH << 3; len -= SHA256_BLOCK_LENGTH; data += SHA256_BLOCK_LENGTH;
@@ -880,7 +881,8 @@
}
while (len >= SHA512_BLOCK_LENGTH) {
/* Process as many complete blocks as we can */
-
SHA512_Transform(context, (sha2_word64*)data);
-
MEMCPY_BCOPY(context->buffer, data, SHA512_BLOCK_LENGTH);
-
SHA512_Transform(context, (sha2_word64*)context->buffer); ADDINC128(context->bitcount, SHA512_BLOCK_LENGTH << 3); len -= SHA512_BLOCK_LENGTH; data += SHA512_BLOCK_LENGTH;
Updated by nahi (Hiroshi Nakamura) over 13 years ago
- Status changed from Assigned to Closed
I applied the last patch to trunk at r32546 and ruby_1_9_3 at r32547.
Nils, sorry for not merging your patch directly. The reason I didn't apply yours is almost from the size of the patch. Would you please report the issue to the upstream (http://www.aarongifford.com/computers/sha.html) for other users, and of course for us?