Opened 4 years ago

Closed 4 years ago

#16970 closed defect (fixed)

Segfault in i386 ed25519_donna_pubkey_from_curve25519_pubkey

Reported by: teor Owned by:
Priority: High Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version: Tor: 0.2.7.2-alpha
Severity: Keywords: Post027Freeze TorCoreTeam201509
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When I run the crypto/ed25519_convert unit test compiled with clang -arch i386 on OS X, it crashes with a segfault in ed25519_donna_pubkey_from_curve25519_pubkey.

crypto/ed25519_convert: 
============================================================ T= 1441245679
Tor 0.2.7.2-alpha-dev died: Caught signal 11
0   test                                0x004b53cf crash_handler + 79
1   libsystem_platform.dylib            0x9c14d03b _sigtramp + 43
2   test                                0x0051f8bc ed25519_donna_pubkey_from_curve25519_pubkey + 380
3   test                                0x004fe56c ed25519_public_key_from_curve25519_public_key + 76
4   test                                0x004fe3d3 ed25519_keypair_from_curve25519_keypair + 451
5   test                                0x0012fece test_crypto_ed25519_convert + 510
6   test                                0x002ecd02 testcase_run_bare_ + 162
7   test                                0x002ec810 testcase_run_one + 432
8   test                                0x002ed507 tinytest_main + 1015
9   test                                0x002ec1b8 main + 984
10  libdyld.dylib                       0x922416d9 start + 1
make: *** [test] Abort trap: 6

Child Tickets

Change History (6)

comment:1 Changed 4 years ago by teor

The segfault occurs on line 146 of curve-25519-donna-sse2.h:

 DONNA_INLINE static void
curve25519_sub(bignum25519 out, const bignum25519 a, const bignum25519 b) {
	xmmi a0,a1,a2,b0,b1,b2;
	xmmi c1,c2;
	xmmi r0,r1;

	a0 = _mm_load_si128((xmmi*)a + 0);
	a1 = _mm_load_si128((xmmi*)a + 1);
	a2 = _mm_load_si128((xmmi*)a + 2);
	a0 = _mm_add_epi32(a0, packed2p0.v);
	a1 = _mm_add_epi32(a1, packed2p1.v);
	a2 = _mm_add_epi32(a2, packed2p2.v);
	b0 = _mm_load_si128((xmmi*)b + 0);

comment:2 Changed 4 years ago by yawning

Apparently clang does something subtly different than gcc with stack alignment, since the jenkins stuff didn't puke here, oh well.

Change ed25519_tor.c:326 to look like this:

  static const bignum25519 ALIGN(16) one = { 1 };

I'd push a branch or whatever but I'm out of the house at the moment and can't be bothered fishing out my smartcard.

comment:3 Changed 4 years ago by nickm

Teor: can you test to confirm that works for you?

comment:4 Changed 4 years ago by nickm

Status: newneeds_review

comment:5 Changed 4 years ago by teor

nickm, yawning: yes, that change works fine. The entire set of unit tests now pass under i386 as well as x86_64, when compiled by Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn) on OS X 10.10.5

comment:6 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Applied this as 81e3deeb54d2f3d022655c6a51f966ef44bf6fb3

Note: See TracTickets for help on using tickets.