Page MenuHome GnuPG

cipher/crc-intel-pclmul.c: load of misaligned address
Closed, ResolvedPublic

Description

Running make check under ASAN (Clang 3.7.1) show some runtime errors,
including the following:

cipher/crc-intel-pclmul.c:347:11: runtime error: load of misaligned address
0x0000005bc501 for type 'const u16' (aka 'const unsigned short'), which requires
2 byte alignment
0x0000005bc501: note: pointer points here
00 00 00 66 6f 6f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00

          ^ 
#0 0x7f68aa3db85d in crc32_reflected_less_than_16

cipher/crc-intel-pclmul.c:347:11

    #1 0x7f68aa3da928 in _gcry_crc32_intel_pclmul cipher/crc-intel-pclmul.c:865:5
    #2 0x7f68aa3d845d in crc32_write cipher/crc.c:385:7
    #3 0x7f68a9f6cc7a in md_write cipher/md.c:612:7
    #4 0x7f68a9f77547 in _gcry_md_hash_buffers cipher/md.c:1045:9
    #5 0x7f68a9eade0f in gcry_md_hash_buffers src/visibility.c:1204:21
    #6 0x523770 in check_one_md_multi tests/basic.c:5638:9
    #7 0x4e37be in check_digests tests/basic.c:6473:7
    #8 0x4db9ce in main tests/basic.c:8934:11
    #9 0x7f68a8a9270f in __libc_start_main (/usr/lib/libc.so.6+0x2070f)
    #10 0x418a38 in _start (tests/basic+0x418a38)

SUMMARY: AddressSanitizer: undefined-behavior cipher/crc-intel-pclmul.c:347:11 in
cipher/crc-intel-pclmul.c:354:11: runtime error: load of misaligned address
0x0000005bc9e1 for type 'const u16' (aka 'const unsigned short'), which requires
2 byte alignment
0x0000005bc9e1: note: pointer points here
00 00 00 80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00

          ^ 
#0 0x7f68aa3db968 in crc32_reflected_less_than_16

cipher/crc-intel-pclmul.c:354:11

    #1 0x7f68aa3da928 in _gcry_crc32_intel_pclmul cipher/crc-intel-pclmul.c:865:5
    #2 0x7f68aa3d845d in crc32_write cipher/crc.c:385:7
    #3 0x7f68a9f6cc7a in md_write cipher/md.c:612:7
    #4 0x7f68a9f77547 in _gcry_md_hash_buffers cipher/md.c:1045:9
    #5 0x7f68a9eade0f in gcry_md_hash_buffers src/visibility.c:1204:21
    #6 0x523770 in check_one_md_multi tests/basic.c:5638:9
    #7 0x4e37be in check_digests tests/basic.c:6473:7
    #8 0x4db9ce in main tests/basic.c:8934:11
    #9 0x7f68a8a9270f in __libc_start_main (/usr/lib/libc.so.6+0x2070f)
    #10 0x418a38 in _start (tests/basic+0x418a38)

SUMMARY: AddressSanitizer: undefined-behavior cipher/crc-intel-pclmul.c:354:11 in
cipher/crc-intel-pclmul.c:711:11: runtime error: load of misaligned address
0x0000005bc501 for type 'const u16' (aka 'const unsigned short'), which requires
2 byte alignment
0x0000005bc501: note: pointer points here
00 00 00 66 6f 6f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00

          ^ 
#0 0x7f68aa3dd0b1 in crc32_less_than_16 cipher/crc-intel-pclmul.c:711:11
#1 0x7f68aa3dc128 in _gcry_crc24rfc2440_intel_pclmul

cipher/crc-intel-pclmul.c:900:5

    #2 0x7f68aa3d92ad in crc24rfc2440_write cipher/crc.c:793:7
    #3 0x7f68a9f6cc7a in md_write cipher/md.c:612:7
    #4 0x7f68a9f77547 in _gcry_md_hash_buffers cipher/md.c:1045:9
    #5 0x7f68a9eade0f in gcry_md_hash_buffers src/visibility.c:1204:21
    #6 0x523770 in check_one_md_multi tests/basic.c:5638:9
    #7 0x4e37be in check_digests tests/basic.c:6473:7
    #8 0x4db9ce in main tests/basic.c:8934:11
    #9 0x7f68a8a9270f in __libc_start_main (/usr/lib/libc.so.6+0x2070f)
    #10 0x418a38 in _start (tests/basic+0x418a38)

SUMMARY: AddressSanitizer: undefined-behavior cipher/crc-intel-pclmul.c:711:11 in

Details

Version
libgcrypt-1.6.0-361-ge709d86

Event Timeline

Lekensteyn set Version to libgcrypt-1.6.0-361-ge709d86.
Lekensteyn added a subscriber: Lekensteyn.

The code in question is only for i386 and given that no SSE instructions are
used I wonder why you consider this a bug. The code might be faster if we would
access the short properly aligned but the question is whether this is worth an
extra copy operation.

jussi: What do you say?

Current code is perfectly fine as crc-intel-pclmul.c is i386/amd64-only source
file and that target architecture can handle unaligned loads.

werner claimed this task.

Thus clang is wrong here because it prints an "error" and not something like a
"performance hint". I close this bug.

I have now learnt how GCC uses 'undefined behavior' for aggressive optimization
and that this could break code doing unaligned accesses even on x86. So this
needs to be fixed after all.