Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

5 messages, 4 authors, 2017-06-01 · open the first message on its own page

Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2017-05-15 12:49:41

Since 2.13.0 just running "git status" on a newly init'd repo on
Solaris SPARC[1] segfaults. Michael (CC'd) reported this issue on #git
and I helped him debug it.

Just compiling with BLK_SHA1=YesPlease solves the issue.

There are at least two different issues with DC_SHA1 here:

 * We don't properly detect that this platform is big endian. The
check at the top of sha1dc/sha1.c needs to test for _BIG_ENDIAN. This
comes from sys/isa_defs.h which (I'm told by #solaris) is included on
Solaris by default, at least by stdio.h.

Hacking the endian detection makes t0013-sha1dc.sh pass.

  * Even with that & the test passing just a plain "git init x && cd x
&& touch A && git add A && git commit" will segfault.

This is some bug in the sha1dc code, presumably some big endian issue
that's not resolved by the change above. Backtrace for that (censored
actual author info):

 Program received signal SIGSEGV, Segmentation fault.
 [Switching to Thread 1 (LWP 1)]
 0x002f8c84 in sha1_compression_states (ihv=0xffbf8268, m=0x398ad5,
W=0xffbf82fc, states=0xffbf857c) at sha1dc/sha1.c:291
 291             SHA1COMPRESS_FULL_ROUND1_STEP_LOAD(a, b, c, d, e, m,
W, 0, temp);
 (gdb) bt
 #0  0x002f8c84 in sha1_compression_states (ihv=0xffbf8268,
m=0x398ad5, W=0xffbf82fc, states=0xffbf857c) at sha1dc/sha1.c:291
 #1  0x00300b60 in sha1_process (ctx=0xffbf8260, block=0x398ad5) at
sha1dc/sha1.c:1616
 #2  0x00301188 in SHA1DCUpdate (ctx=0xffbf8260,
     buf=0x398ad5 "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\nauthor Au
Thor [off-list ref] 123456789 +0000\ncommitter Au Thor
[off-list ref] 123456789
                  +0000\n\nBlah Blah"..., len=220)
     at sha1dc/sha1.c:1731
 #3  0x0030168c in git_SHA1DCUpdate (ctx=0xffbf8260, vdata=0x398aa0,
len=273) at sha1dc/sha1.c:1808
 #4  0x002a6f7c in write_sha1_file_prepare (buf=0x398aa0, len=273,
type=0x959c8 "commit", sha1=0xffbfd630 "",
     hdr=0xffbf8c28 "commit 273", hdrlen=0xffbf8c24) at sha1_file.c:3207
 #5  0x002a71ac in hash_sha1_file (buf=0x398aa0, len=273, type=0x959c8
"commit", sha1=0xffbfd630 "") at sha1_file.c:3266
 #6  0x002a25f8 in check_sha1_signature (sha1=0xffbfdbb8
"\375\067\356\337\002", map=0x398aa0, size=273, type=0x959c8 "commit")
     at sha1_file.c:1644
 #7  0x0022816c in parse_object (sha1=0xffbfdbb8
"\375\067\356\337\002") at object.c:269
 #8  0x0027c258 in get_reference (revs=0xffbfdc88, name=0xa87f0
"HEAD", sha1=0xffbfdbb8 "\375\067\356\337\002", flags=0)
     at revision.c:196
 #9  0x00284714 in setup_revisions (argc=0, argv=0x0, revs=0xffbfdc88,
opt=0xffbfdc74) at revision.c:2295
 #10 0x002ee4d8 in wt_status_collect_changes_index (s=0x348ea8
<s.24114>) at wt-status.c:585
 #11 0x002eeae8 in wt_status_collect (s=0x348ea8 <s.24114>) at wt-status.c:701
 #12 0x000db4fc in cmd_status (argc=0, argv=0xffbfe6fc, prefix=0x0) at
builtin/commit.c:1393
 #13 0x000acc24 in run_builtin (p=0x340ccc <commands+1200>, argc=1,
argv=0xffbfe6fc) at git.c:371
 #14 0x000acf98 in handle_builtin (argc=1, argv=0xffbfe6fc) at git.c:572
 #15 0x000ad1a4 in run_argv (argcp=0xffbfe66c, argv=0xffbfe670) at git.c:624
 #16 0x000ad3fc in cmd_main (argc=1, argv=0xffbfe6fc) at git.c:701
 #17 0x001747a0 in main (argc=5, argv=0xffbfe6ec) at common-main.c:43

1. Relevant part of uname -a: "SunOS <hostname> 5.11 11.3 sun4v sparc sun4v".

RE: Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

From: Marc Stevens <hidden>
Date: 2017-05-15 13:59:06

Hi Aevar,

Thank you for notifying us of this issue.
Big endianness is a tricky issue, also since I don't have access or accurate knowledge about all big endian systems.
Our github repo does check correct functioning, including an endianness mistake, with 'make test'.
But I guess this is not included for SHA1DC in Git.

Anyway, we can easily add the _BIG_ENDIAN macrotest to the git repo and will do so soon.

I don't think the segfault is caused by buffer overflow, inproper access, or the endianness issue.
But I did notice an unexpected issue: the message block pointer m=0x398ad5 is odd. 
Can you confirm whether loading an uint32_t from an odd address triggers a hardware interrupt on your platform?
This is not problem for x86, but maybe for your platform it is?
If it is then we should always copy buffer contents to the sha1context to avoid this issue.

Best regards,
Marc Stevens

-----Oorspronkelijk bericht-----
Van: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com] 
Verzonden: maandag 15 mei 2017 14:49
Aan: Git Mailing List [off-list ref]
CC: michael.kebe@gmail.com; Jeff King <redacted>; Marc Stevens <redacted>
Onderwerp: Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

Since 2.13.0 just running "git status" on a newly init'd repo on Solaris SPARC[1] segfaults. Michael (CC'd) reported this issue on #git and I helped him debug it.

Just compiling with BLK_SHA1=YesPlease solves the issue.

There are at least two different issues with DC_SHA1 here:

 * We don't properly detect that this platform is big endian. The check at the top of sha1dc/sha1.c needs to test for _BIG_ENDIAN. This comes from sys/isa_defs.h which (I'm told by #solaris) is included on Solaris by default, at least by stdio.h.

Hacking the endian detection makes t0013-sha1dc.sh pass.

  * Even with that & the test passing just a plain "git init x && cd x && touch A && git add A && git commit" will segfault.

This is some bug in the sha1dc code, presumably some big endian issue that's not resolved by the change above. Backtrace for that (censored actual author info):

 Program received signal SIGSEGV, Segmentation fault.
 [Switching to Thread 1 (LWP 1)]
 0x002f8c84 in sha1_compression_states (ihv=0xffbf8268, m=0x398ad5, W=0xffbf82fc, states=0xffbf857c) at sha1dc/sha1.c:291
 291             SHA1COMPRESS_FULL_ROUND1_STEP_LOAD(a, b, c, d, e, m,
W, 0, temp);
 (gdb) bt
 #0  0x002f8c84 in sha1_compression_states (ihv=0xffbf8268, m=0x398ad5, W=0xffbf82fc, states=0xffbf857c) at sha1dc/sha1.c:291
 #1  0x00300b60 in sha1_process (ctx=0xffbf8260, block=0x398ad5) at
sha1dc/sha1.c:1616
 #2  0x00301188 in SHA1DCUpdate (ctx=0xffbf8260,
     buf=0x398ad5 "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\nauthor Au Thor [off-list ref] 123456789 +0000\ncommitter Au Thor [off-list ref] 123456789
                  +0000\n\nBlah Blah"..., len=220)
     at sha1dc/sha1.c:1731
 #3  0x0030168c in git_SHA1DCUpdate (ctx=0xffbf8260, vdata=0x398aa0,
len=273) at sha1dc/sha1.c:1808
 #4  0x002a6f7c in write_sha1_file_prepare (buf=0x398aa0, len=273,
type=0x959c8 "commit", sha1=0xffbfd630 "",
     hdr=0xffbf8c28 "commit 273", hdrlen=0xffbf8c24) at sha1_file.c:3207
 #5  0x002a71ac in hash_sha1_file (buf=0x398aa0, len=273, type=0x959c8 "commit", sha1=0xffbfd630 "") at sha1_file.c:3266
 #6  0x002a25f8 in check_sha1_signature (sha1=0xffbfdbb8 "\375\067\356\337\002", map=0x398aa0, size=273, type=0x959c8 "commit")
     at sha1_file.c:1644
 #7  0x0022816c in parse_object (sha1=0xffbfdbb8
"\375\067\356\337\002") at object.c:269
 #8  0x0027c258 in get_reference (revs=0xffbfdc88, name=0xa87f0 "HEAD", sha1=0xffbfdbb8 "\375\067\356\337\002", flags=0)
     at revision.c:196
 #9  0x00284714 in setup_revisions (argc=0, argv=0x0, revs=0xffbfdc88,
opt=0xffbfdc74) at revision.c:2295
 #10 0x002ee4d8 in wt_status_collect_changes_index (s=0x348ea8
<s.24114>) at wt-status.c:585
 #11 0x002eeae8 in wt_status_collect (s=0x348ea8 <s.24114>) at wt-status.c:701
 #12 0x000db4fc in cmd_status (argc=0, argv=0xffbfe6fc, prefix=0x0) at
builtin/commit.c:1393
 #13 0x000acc24 in run_builtin (p=0x340ccc <commands+1200>, argc=1,
argv=0xffbfe6fc) at git.c:371
 #14 0x000acf98 in handle_builtin (argc=1, argv=0xffbfe6fc) at git.c:572
 #15 0x000ad1a4 in run_argv (argcp=0xffbfe66c, argv=0xffbfe670) at git.c:624
 #16 0x000ad3fc in cmd_main (argc=1, argv=0xffbfe6fc) at git.c:701
 #17 0x001747a0 in main (argc=5, argv=0xffbfe6ec) at common-main.c:43

1. Relevant part of uname -a: "SunOS <hostname> 5.11 11.3 sun4v sparc sun4v".

Re: Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2017-05-15 14:14:25

On Mon, May 15, 2017 at 3:58 PM, Marc Stevens [off-list ref] wrote:
Hi Aevar,

Thank you for notifying us of this issue.
Big endianness is a tricky issue, also since I don't have access or accurate knowledge about all big endian systems.
Our github repo does check correct functioning, including an endianness mistake, with 'make test'.
But I guess this is not included for SHA1DC in Git.

Anyway, we can easily add the _BIG_ENDIAN macrotest to the git repo and will do so soon.

I don't think the segfault is caused by buffer overflow, inproper access, or the endianness issue.
But I did notice an unexpected issue: the message block pointer m=0x398ad5 is odd.
Can you confirm whether loading an uint32_t from an odd address triggers a hardware interrupt on your platform?
This is not problem for x86, but maybe for your platform it is?
If it is then we should always copy buffer contents to the sha1context to avoid this issue.
I don't have access to the box in question, Michael was testing this
code for me. But unaligned access is probably the cause, although
according to some info I found online that should give a SIGBUS not a
SIGSEGV, but that may have changed:

https://bugs.python.org/issue12181
https://github.com/magnumripper/JohnTheRipper/issues/2187
Best regards,
Marc Stevens

-----Oorspronkelijk bericht-----
Van: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com]
Verzonden: maandag 15 mei 2017 14:49
Aan: Git Mailing List [off-list ref]
CC: michael.kebe@gmail.com; Jeff King <redacted>; Marc Stevens <redacted>
Onderwerp: Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

Since 2.13.0 just running "git status" on a newly init'd repo on Solaris SPARC[1] segfaults. Michael (CC'd) reported this issue on #git and I helped him debug it.

Just compiling with BLK_SHA1=YesPlease solves the issue.

There are at least two different issues with DC_SHA1 here:

 * We don't properly detect that this platform is big endian. The check at the top of sha1dc/sha1.c needs to test for _BIG_ENDIAN. This comes from sys/isa_defs.h which (I'm told by #solaris) is included on Solaris by default, at least by stdio.h.

Hacking the endian detection makes t0013-sha1dc.sh pass.

  * Even with that & the test passing just a plain "git init x && cd x && touch A && git add A && git commit" will segfault.

This is some bug in the sha1dc code, presumably some big endian issue that's not resolved by the change above. Backtrace for that (censored actual author info):

 Program received signal SIGSEGV, Segmentation fault.
 [Switching to Thread 1 (LWP 1)]
 0x002f8c84 in sha1_compression_states (ihv=0xffbf8268, m=0x398ad5, W=0xffbf82fc, states=0xffbf857c) at sha1dc/sha1.c:291
 291             SHA1COMPRESS_FULL_ROUND1_STEP_LOAD(a, b, c, d, e, m,
W, 0, temp);
 (gdb) bt
 #0  0x002f8c84 in sha1_compression_states (ihv=0xffbf8268, m=0x398ad5, W=0xffbf82fc, states=0xffbf857c) at sha1dc/sha1.c:291
 #1  0x00300b60 in sha1_process (ctx=0xffbf8260, block=0x398ad5) at
sha1dc/sha1.c:1616
 #2  0x00301188 in SHA1DCUpdate (ctx=0xffbf8260,
     buf=0x398ad5 "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\nauthor Au Thor [off-list ref] 123456789 +0000\ncommitter Au Thor [off-list ref] 123456789
                  +0000\n\nBlah Blah"..., len=220)
     at sha1dc/sha1.c:1731
 #3  0x0030168c in git_SHA1DCUpdate (ctx=0xffbf8260, vdata=0x398aa0,
len=273) at sha1dc/sha1.c:1808
 #4  0x002a6f7c in write_sha1_file_prepare (buf=0x398aa0, len=273,
type=0x959c8 "commit", sha1=0xffbfd630 "",
     hdr=0xffbf8c28 "commit 273", hdrlen=0xffbf8c24) at sha1_file.c:3207
 #5  0x002a71ac in hash_sha1_file (buf=0x398aa0, len=273, type=0x959c8 "commit", sha1=0xffbfd630 "") at sha1_file.c:3266
 #6  0x002a25f8 in check_sha1_signature (sha1=0xffbfdbb8 "\375\067\356\337\002", map=0x398aa0, size=273, type=0x959c8 "commit")
     at sha1_file.c:1644
 #7  0x0022816c in parse_object (sha1=0xffbfdbb8
"\375\067\356\337\002") at object.c:269
 #8  0x0027c258 in get_reference (revs=0xffbfdc88, name=0xa87f0 "HEAD", sha1=0xffbfdbb8 "\375\067\356\337\002", flags=0)
     at revision.c:196
 #9  0x00284714 in setup_revisions (argc=0, argv=0x0, revs=0xffbfdc88,
opt=0xffbfdc74) at revision.c:2295
 #10 0x002ee4d8 in wt_status_collect_changes_index (s=0x348ea8
<s.24114>) at wt-status.c:585
 #11 0x002eeae8 in wt_status_collect (s=0x348ea8 <s.24114>) at wt-status.c:701
 #12 0x000db4fc in cmd_status (argc=0, argv=0xffbfe6fc, prefix=0x0) at
builtin/commit.c:1393
 #13 0x000acc24 in run_builtin (p=0x340ccc <commands+1200>, argc=1,
argv=0xffbfe6fc) at git.c:371
 #14 0x000acf98 in handle_builtin (argc=1, argv=0xffbfe6fc) at git.c:572
 #15 0x000ad1a4 in run_argv (argcp=0xffbfe66c, argv=0xffbfe670) at git.c:624
 #16 0x000ad3fc in cmd_main (argc=1, argv=0xffbfe6fc) at git.c:701
 #17 0x001747a0 in main (argc=5, argv=0xffbfe6ec) at common-main.c:43

1. Relevant part of uname -a: "SunOS <hostname> 5.11 11.3 sun4v sparc sun4v".

Re: Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

From: Jeff King <hidden>
Date: 2017-05-15 22:09:48

On Mon, May 15, 2017 at 04:13:58PM +0200, Ævar Arnfjörð Bjarmason wrote:
On Mon, May 15, 2017 at 3:58 PM, Marc Stevens [off-list ref] wrote:
quoted
Hi Aevar,

Thank you for notifying us of this issue.
Big endianness is a tricky issue, also since I don't have access or accurate knowledge about all big endian systems.
Our github repo does check correct functioning, including an endianness mistake, with 'make test'.
But I guess this is not included for SHA1DC in Git.

Anyway, we can easily add the _BIG_ENDIAN macrotest to the git repo and will do so soon.

I don't think the segfault is caused by buffer overflow, inproper access, or the endianness issue.
But I did notice an unexpected issue: the message block pointer m=0x398ad5 is odd.
Can you confirm whether loading an uint32_t from an odd address triggers a hardware interrupt on your platform?
This is not problem for x86, but maybe for your platform it is?
If it is then we should always copy buffer contents to the sha1context to avoid this issue.
I don't have access to the box in question, Michael was testing this
code for me. But unaligned access is probably the cause, although
according to some info I found online that should give a SIGBUS not a
SIGSEGV, but that may have changed:
Yeah, I would have expected SIGBUS there. If we have alignment issues,
though, I'd expect that ARM systems will experience problems.

Block-sha1 uses a macro which allows unaligned loads on platforms that
support it, and otherwise does the endian conversion on the fly as we
load the bytes into a local variable (which presumably happens all
in-register). That may be faster than doing a mass copy of the buffer.

-Peff

Re: Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

From: demerphq <hidden>
Date: 2017-06-01 14:03:15

On 16 May 2017 at 00:09, Jeff King [off-list ref] wrote:
On Mon, May 15, 2017 at 04:13:58PM +0200, Ævar Arnfjörð Bjarmason wrote:
quoted
On Mon, May 15, 2017 at 3:58 PM, Marc Stevens [off-list ref] wrote:
quoted
Hi Aevar,

Thank you for notifying us of this issue.
Big endianness is a tricky issue, also since I don't have access or accurate knowledge about all big endian systems.
Our github repo does check correct functioning, including an endianness mistake, with 'make test'.
But I guess this is not included for SHA1DC in Git.

Anyway, we can easily add the _BIG_ENDIAN macrotest to the git repo and will do so soon.

I don't think the segfault is caused by buffer overflow, inproper access, or the endianness issue.
But I did notice an unexpected issue: the message block pointer m=0x398ad5 is odd.
Can you confirm whether loading an uint32_t from an odd address triggers a hardware interrupt on your platform?
This is not problem for x86, but maybe for your platform it is?
If it is then we should always copy buffer contents to the sha1context to avoid this issue.
I don't have access to the box in question, Michael was testing this
code for me. But unaligned access is probably the cause, although
according to some info I found online that should give a SIGBUS not a
SIGSEGV, but that may have changed:
Yeah, I would have expected SIGBUS there. If we have alignment issues,
though, I'd expect that ARM systems will experience problems.

Block-sha1 uses a macro which allows unaligned loads on platforms that
support it, and otherwise does the endian conversion on the fly as we
load the bytes into a local variable (which presumably happens all
in-register). That may be faster than doing a mass copy of the buffer.
I agree. It is fairly normal to use that kind of macro and not do a
memcpy with hash functions.

In fact many hash functions ONLY use that kind of macro, as decent
compilers will automagically convert the macro into an unaligned load
on platforms that support fast unaligned loads.

The only reason to expose the direct unaligned load is to make sure
that the hashing code is fast on such platforms even when compiled
under -g.

Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help