From ba831301ddd3c1f7600163cf00bbe48d5fefedf1 Mon Sep 17 00:00:00 2001 From: doomfrawen Date: Sun, 9 Aug 2026 09:24:59 -0700 Subject: [PATCH 1/2] Fix Windows/MinGW build: missing err.h, uninitialized pointer in stdlib.c - src/nfc-utils.h/.c and src/windows.h include , a BSD/glibc-only header not shipped by MinGW-w64, so the GNU-toolchain build (autoreconf + gcc, as opposed to the VS2019/clang-cl path this repo also supports) fails with 'err.h: No such file or directory'. windows.h doesn't call anything from it, so that include is simply dropped. nfc-utils.h's DBG/WARN/ERR macros do use warnx(), so on _WIN32 this adds a small static inline warnx() shim instead of the header, and drops the redundant second #include in nfc-utils.c. - src/stdlib.c's setenv()/unsetenv() declare 'char *str[32]' (an array of 32 uninitialized pointers) and then do strcpy(*str, name) etc, which dereferences the first (uninitialized/garbage) pointer in that array and writes through it - undefined behavior, not just a compile-time type mismatch. Changed to 'char str[32]' (an actual 32-byte buffer) and dropped the erroneous dereferences. --- src/nfc-utils.c | 1 - src/nfc-utils.h | 15 ++++++++++++++- src/stdlib.c | 18 +++++++++--------- src/windows.h | 1 - 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/nfc-utils.c b/src/nfc-utils.c index 5c0a264..f5b13b1 100644 --- a/src/nfc-utils.c +++ b/src/nfc-utils.c @@ -37,7 +37,6 @@ * @brief Provide some examples shared functions like print, parity calculation, options parsing. */ #include -#include #include "nfc-utils.h" diff --git a/src/nfc-utils.h b/src/nfc-utils.h index a0a599e..b46b019 100644 --- a/src/nfc-utils.h +++ b/src/nfc-utils.h @@ -43,7 +43,20 @@ # include # include -# include +# ifdef _WIN32 +# include +# include +static inline void warnx(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + fprintf(stderr, "\n"); +} +# else +# include +# endif # include /** diff --git a/src/stdlib.c b/src/stdlib.c index 9a0a512..0dfa02f 100644 --- a/src/stdlib.c +++ b/src/stdlib.c @@ -40,19 +40,19 @@ int setenv(const char *name, const char *value, int overwrite) { char *env = getenv(name); if ((env && overwrite) || (!env)) { - char *str[32]; - strcpy(*str, name); - strcat(*str, "="); - strcat(*str, value); - return _putenv(*str); + char str[32]; + strcpy(str, name); + strcat(str, "="); + strcat(str, value); + return _putenv(str); } return -1; } void unsetenv(const char *name) { - char *str[32]; - strcpy(*str, name); - strcat(*str, "="); - _putenv(*str); + char str[32]; + strcpy(str, name); + strcat(str, "="); + _putenv(str); } diff --git a/src/windows.h b/src/windows.h index 2a9e722..134ca99 100644 --- a/src/windows.h +++ b/src/windows.h @@ -35,7 +35,6 @@ # include # include -# include # if defined (__MINGW32__) /* * Cheating here on the snprintf to incorporate the format argument From f017c5223993dea9ee1425be5f0761ab68262ff8 Mon Sep 17 00:00:00 2001 From: doomfrawen Date: Wed, 19 Aug 2026 11:53:33 -0700 Subject: [PATCH 2/2] Fix -Z (low-memory) segfault: stack alignment + use-after-free race -mstackrealign on the AVX/AVX2/AVX512 hardnested cores: MinGW-w64 only guarantees a 16-byte aligned stack, but code built for these targets can spill 32-byte YMM values to it. Without forced realignment that's a segfault. That alone didn't fix -Z (hard_low_memory) mode, though: it still crashed with a use-after-free, confirmed via gdb backtrace landing in count_bitarray_AND_AVX2 on a table check_for_BitFlipProperties_thread had just fetched via get_bitflip_data(). Root cause: -Z decompresses each bitflip property table on demand and frees it right after use via remove_bitflip_data(), but check_for_BitFlipProperties() spawns one thread per CPU that all walk the same bitflip index sequence over disjoint nonce ranges - so multiple threads can legitimately hold the same (odd_even, bitflip) table at once. One thread finishing and freeing it out from under another thread still reading it is a straight use-after-free. Fixed by reference-counting instead of freeing on every use: get_bitflip_data() takes a reference under the same lock that guards allocation, remove_bitflip_data() only frees once the count drops to zero. Also fixed both call sites in cmdhfmfhard.c to acquire the pointer once and reuse it, instead of calling get_bitflip_data() twice per use against one remove_bitflip_data() release, which would have left the refcount permanently inflated. --- src/Makefile.am | 10 +++++++--- src/cmdhfmfhard.c | 42 ++++++++++++++++++++++++++++++------------ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 2d517fa..36b6609 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,10 +9,14 @@ mfoc_hardnested_LDADD = @libnfc_LIBS@ $(SIMD) dist_man_MANS = mfoc-hardnested.1 +# -mstackrealign on the AVX/AVX2/AVX512 cores: MinGW-w64 only guarantees a +# 16-byte aligned stack, but code compiled for these targets can spill 32-byte +# YMM values to the stack. Without forced realignment that's a segfault, +# reliably hit inside the pthread worker threads spawned for the brute force. HARD_SWITCH_SSE2 = -mmmx -msse2 -mno-avx -mno-avx2 -mno-avx512f -HARD_SWITCH_AVX = -mmmx -msse2 -mavx -mno-avx2 -mno-avx512f -HARD_SWITCH_AVX2 = -mmmx -msse2 -mavx -mavx2 -mno-avx512f -HARD_SWITCH_AVX512 = -mmmx -msse2 -mavx -mavx2 -mavx512f +HARD_SWITCH_AVX = -mmmx -msse2 -mavx -mno-avx2 -mno-avx512f -mstackrealign +HARD_SWITCH_AVX2 = -mmmx -msse2 -mavx -mavx2 -mno-avx512f -mstackrealign +HARD_SWITCH_AVX512 = -mmmx -msse2 -mavx -mavx2 -mavx512f -mstackrealign if X86_SIMD diff --git a/src/cmdhfmfhard.c b/src/cmdhfmfhard.c index c5f792c..375c7bc 100644 --- a/src/cmdhfmfhard.c +++ b/src/cmdhfmfhard.c @@ -108,14 +108,28 @@ static uint8_t targetKEY; static bool hard_LOW_MEM; static bool bitflips_available[2][0x400]; static bool bitflips_allocated[2][0x400]; +// -Z mode's worker threads all walk the same bitflip index sequence over +// disjoint nonce ranges, so several threads can hold a table returned by +// get_bitflip_data() for the same (odd_even, bitflip) at once. Refcounting +// (instead of freeing unconditionally on every remove_bitflip_data() call) +// keeps a thread that's still reading the table from losing it to another +// thread's free - a use-after-free that otherwise segfaults inside the +// bitsliced AND cores. +static uint16_t bitflip_refcount[2][0x400]; static pthread_mutex_t bitflip_mutex = PTHREAD_MUTEX_INITIALIZER; void remove_bitflip_data(odd_even_t odd_even, uint16_t bitflip){ pthread_mutex_lock(&bitflip_mutex); if (hard_LOW_MEM && bitflips_allocated[odd_even][bitflip]) { - free_bitarray(bitflip_bitarrays[odd_even][bitflip]); - bitflips_allocated[odd_even][bitflip] = false; + if (bitflip_refcount[odd_even][bitflip] > 0) { + bitflip_refcount[odd_even][bitflip]--; + } + if (bitflip_refcount[odd_even][bitflip] == 0) { + free_bitarray(bitflip_bitarrays[odd_even][bitflip]); + bitflip_bitarrays[odd_even][bitflip] = NULL; + bitflips_allocated[odd_even][bitflip] = false; + } } pthread_mutex_unlock(&bitflip_mutex); } @@ -150,11 +164,13 @@ uint32_t* get_bitflip_data(odd_even_t odd_even, uint16_t bitflip) { } lzma_end(&strm); } + uint32_t *result = bitflip_bitarrays[odd_even][bitflip]; + if (hard_LOW_MEM && result != NULL) { + bitflip_refcount[odd_even][bitflip]++; + } pthread_mutex_unlock(&bitflip_mutex); - - return bitflip_bitarrays[odd_even][bitflip]; - + return result; } @@ -993,14 +1009,15 @@ __attribute__((force_align_arg_pointer)) || (parity1 != parity2 && (bitflip & 0x100))) { // not bitflip nonces[i].BitFlips[bitflip] = 1; for (odd_even_t odd_even = EVEN_STATE; odd_even <= ODD_STATE; odd_even++) { - if (get_bitflip_data(odd_even, bitflip) != NULL) { + uint32_t *bitflip_data = get_bitflip_data(odd_even, bitflip); + if (bitflip_data != NULL) { uint32_t old_count = nonces[i].num_states_bitarray[odd_even]; - nonces[i].num_states_bitarray[odd_even] = count_bitarray_AND(nonces[i].states_bitarray[odd_even], get_bitflip_data(odd_even, bitflip)); + nonces[i].num_states_bitarray[odd_even] = count_bitarray_AND(nonces[i].states_bitarray[odd_even], bitflip_data); if (nonces[i].num_states_bitarray[odd_even] != old_count) { nonces[i].all_bitflips_dirty[odd_even] = true; } - } - remove_bitflip_data(odd_even, bitflip); + remove_bitflip_data(odd_even, bitflip); + } } } } @@ -1029,14 +1046,15 @@ __attribute__((force_align_arg_pointer)) || (parity1 != parity2 && (bitflip & 0x100))) { // not bitflip nonces[i].BitFlips[bitflip] = 1; for (odd_even_t odd_even = EVEN_STATE; odd_even <= ODD_STATE; odd_even++) { - if (get_bitflip_data(odd_even, bitflip) != NULL) { + uint32_t *bitflip_data = get_bitflip_data(odd_even, bitflip); + if (bitflip_data != NULL) { uint32_t old_count = nonces[i].num_states_bitarray[odd_even]; - nonces[i].num_states_bitarray[odd_even] = count_bitarray_AND(nonces[i].states_bitarray[odd_even], get_bitflip_data(odd_even, bitflip)); + nonces[i].num_states_bitarray[odd_even] = count_bitarray_AND(nonces[i].states_bitarray[odd_even], bitflip_data); if (nonces[i].num_states_bitarray[odd_even] != old_count) { nonces[i].all_bitflips_dirty[odd_even] = true; } + remove_bitflip_data(odd_even, bitflip); } - remove_bitflip_data(odd_even, bitflip); } break; }