Opened 5 years ago

Closed 5 years ago

#14982 closed defect (worksforme)

fails to establish connections due to gcc 5.0 bug/feature with -O3

Reported by: Safari Owned by:
Priority: High Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version: Tor: 0.2.6.3-alpha
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

gcc 5.0 with -O3 seems to compile curve25519-donna-c64.c so that curve25519_donna writes all-zeros to mypublic.
This is caught by "make test", but not when starting tor, I hope this check is added to tor on startup.

When one just starts tor, debug logs point nowhere, because they assume curve25519 works OK.

ntor_handshake:

FAIL src/test/test.c:238: assert(0 OP_EQ onion_skin_ntor_server_handshake(c_buf, s_keymap, NULL, node_id, s_buf, s_keys, 400)): 0 vs -1
[ntor_handshake FAILED]

crypto/curve25519_impl:

FAIL src/test/test_crypto.c:1018: assert(e1 == mem_op_hex_tmp): 0300000000000000000000000000000000000000000000000000000000000000 vs BC7112CDE03F97EF7008CAD1BDC56BE3C6A1037D74CCEB3712E9206871DCF654
[curve25519_impl FAILED]

crypto/curve25519_impl_hibit:

FAIL src/test/test_crypto.c:1018: assert(e1 == mem_op_hex_tmp): 0300000000000000000000000000000000000000000000000000000000000000 vs BC7112CDE03F97EF7008CAD1BDC56BE3C6A1037D74CCEB3712E9206871DCF654
[curve25519_impl_hibit FAILED]

crypto/curve25519_wrappers:

FAIL src/test/test_crypto.c:1041: assert(curve25519_public_key_is_ok(&pubkey1))
[curve25519_wrappers FAILED]

crypto/ed25519_convert: [err] tor_assertion_failed_(): Bug: src/common/crypto_ed25519.c:214: ed25519_keypair_from_curve25519_keypair: Assertion fast_memeq(pubkey_check.pubkey, out->pubkey.pubkey, 32) failed; aborting.
[err] Bug: Assertion fast_memeq(pubkey_check.pubkey, out->pubkey.pubkey, 32) failed in ed25519_keypair_from_curve25519_keypair at src/common/crypto_ed25519.c:214. Stack trace:

Child Tickets

Change History (5)

comment:1 Changed 5 years ago by yawning

This looks like a GCC bug, someone should check to see if they know about it and if not, file one.

I'm not sure if "add a ton of internal tests to see if the compiler generated correct code" makes a whole lot of sense, since as you noted, the issue is caught by the test suite. There's no guarantee that the rest of the code is actually being compiled correctly either at this point...

My initial thoughts on this matter are to add a workaround if GCC ships an actual release with this problem. Otherwise, anyone that braves using a development snapshot compiler (or builds tor with non-standard CFLAGS) should be running unit tests.

It's a nickm decision though, so hopefully he sees this soon.

comment:2 Changed 5 years ago by nickm

I'm inclined to agree with Yawning: This should be reported to the compiler people, if they don't know already. Holding the bug open till there's a link to the bug report, so nobody forgets.

But if you are using a snapshot compiler, you really do need to run the tests on everything you build.

comment:3 Changed 5 years ago by nickm

Status: newneeds_information

comment:4 Changed 5 years ago by Safari

Jakub was quick to find the first buggy revision of gcc and tested fix should be soon available
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65170

comment:5 Changed 5 years ago by nickm

Resolution: worksforme
Status: needs_informationclosed

Excellent; we can call this a gcc bug then.

Note: See TracTickets for help on using tickets.