technion/libscrypt

Test failure on macOS with Xcode 15 / clang 15

Opened this issue · 5 comments

We compile with:

make install-osx PREFIX=/opt/homebrew/Cellar/libscrypt/1.22 LDFLAGS= LDFLAGS_EXTRA= CFLAGS_EXTRA=

then run checks with:

$ make check LDFLAGS= LDFLAGS_EXTRA= CFLAGS_EXTRA=
LD_LIBRARY_PATH=. ./reference
TEST ONE: Direct call to reference function with password 'password' and salt 'NaCL'
TEST ONE: SUCCESSFUL
TEST ONE and a half: Review errno on invalid input
TEST ONE and a half: Successfully failed on error: Invalid argument
TEST TWO: Convert binary output to hex
TEST TWO: SUCCESSFUL, Hex output is:
06f8551fcbac81156d42f53707eea6a74ae4c344f5f03be9340da145cb4ebe0a129f587fb2fcf211da5276c6fd9b29c1eb1995145eac7f23e211070412a1498c
TEST THREE: Compare hex output to reference hash output
TEST THREE: FAILED to match reference on hash
make: *** [check] Error 1

The check passes with clang 14

Thanks for the report. Unfortunately I don't have any Mac hardware and I'm pretty much down to someone from the community suggesting a fix here.

OK, we (at Homebrew) have reduced it to a difference in behaviour between -Os and -O2 in clang 15.

  • Compiling at -O2, the test passes:
clang -O2 crypto_scrypt-nosse.c sha256.c crypto-mcf.c b64.c crypto-scrypt-saltgen.c crypto_scrypt-check.c crypto_scrypt-hash.c slowequals.c main.c crypto_scrypt-hexconvert.c -c -w && clang *.o && ./a.out
  • Compiling at -Os, the test fails.
clang -Os crypto_scrypt-nosse.c sha256.c crypto-mcf.c b64.c crypto-scrypt-saltgen.c crypto_scrypt-check.c crypto_scrypt-hash.c slowequals.c main.c crypto_scrypt-hexconvert.c -c -w && clang *.o && ./a.out

Going further, if you compile all source with -O2 and only crypto_scrypt-nosse.c with -Os then the test fails:

clang -O2 sha256.c crypto-mcf.c b64.c crypto-scrypt-saltgen.c crypto_scrypt-check.c crypto_scrypt-hash.c slowequals.c main.c crypto_scrypt-hexconvert.c -c -w && clang -Os crypto_scrypt-nosse.c -c && clang *.o && ./a.out

So it appears the miscompiled source is in crypto_scrypt-nosse.c.

I have reported this to Apple as FB13546010

The issue is with the code generation for the blkcopy function. I can verify this in two ways, because the bug goes away if I either:

  • define blkcpy as a macro equivalent of memmove
  • add __attribute__ ((optnone)) to the blkcpy definition

The code for the function is the following:

void
blkcpy(void * dest, void * src, size_t len)
{
        size_t * D = dest;
        size_t * S = src;
        size_t L = len / sizeof(size_t);
        size_t i;

        for (i = 0; i < L; i++)
                D[i] = S[i];
}

I think it's legal, as long as len is always a multiple of size_t, which a cursory audit seems to indicate is the case.


Edit: I can narrow it further, it's triggered by the call to blkcpy from salsa20_8. All other calls can be replaced by memmove and don't make the issue go away.

I've found the exact same problem on Gentoo (and then just in a clone of this repo). Compiling with GCC 13.2.1 with -O3 produces the problem, but -O2 does not. A cursory test on Compiler Explorer suggests blkcpy is optimised like GCC13 -O3 with Clang17 -O2.
I'm not quite so convinced the issue is with blkcpy though, as adding a check that all calls are with len which is a multiple of sizeof(size_t)... and from a quick grep, all calls are with len 64 or some multiple of 128.
Which I'm afraid leaves me and no idea where the problem is... but hopefully given the above someone can narrow it down.

 ~/thirdparty/libscrypt   master  CFLAGS="-O3" make -B check
cc -O3   -c -o crypto_scrypt-nosse.o crypto_scrypt-nosse.c
cc -O3   -c -o sha256.o sha256.c
cc -O3   -c -o crypto-mcf.o crypto-mcf.c
cc -O3   -c -o b64.o b64.c
cc -O3   -c -o crypto-scrypt-saltgen.o crypto-scrypt-saltgen.c
cc -O3   -c -o crypto_scrypt-check.o crypto_scrypt-check.c
cc -O3   -c -o crypto_scrypt-hash.o crypto_scrypt-hash.c
cc -O3   -c -o slowequals.o slowequals.c
cc -Wl,-z,relro -Wl,-soname,libscrypt.so.0 -Wl,--version-script=libscrypt.version -shared -o libscrypt.so.0  crypto_scrypt-nosse.o sha256.o crypto-mcf.o b64.o crypto-scrypt-saltgen.o crypto_scrypt-check.o crypto_scrypt-hash.o slowequals.o -lm -lc
ar rcs libscrypt.a  crypto_scrypt-nosse.o sha256.o crypto-mcf.o b64.o crypto-scrypt-saltgen.o crypto_scrypt-check.o crypto_scrypt-hash.o slowequals.o
cc -O3   -c -o main.o main.c
cc -O3   -c -o crypto_scrypt-hexconvert.o crypto_scrypt-hexconvert.c
ln -s -f libscrypt.so.0 libscrypt.so
cc -o reference main.o b64.o crypto_scrypt-hexconvert.o -O3 -Wl,-z,relro -L.  -lscrypt
LD_LIBRARY_PATH=. ./reference
TEST ONE: Direct call to reference function with password 'password' and salt 'NaCL'
TEST ONE: SUCCESSFUL
TEST ONE and a half: Review errno on invalid input
TEST ONE and a half: Successfully failed on error: Invalid argument
TEST TWO: Convert binary output to hex
TEST TWO: SUCCESSFUL, Hex output is:
fa3fb34a9e7282b1908159da930c840f8fd08239610a793639a25e81155e8d1a5a64afc95a7655cc988263f9a7b4d6a788552eaea2bebc8d51ba0835ff370ca8
TEST THREE: Compare hex output to reference hash output
TEST THREE: FAILED to match reference on hash
make: *** [Makefile:29: check] Error 1

Of note: GCC 10.5.0 is fine with -O3 however GCC 12.3.1 is not... I don't have a GCC 11 to hand to test with.