tukaani-project / xz

XZ Utils
https://tukaani.org/xz/
Other
561 stars 104 forks source link

tsan also needs sanitizer nerf for crc64 #122

Closed nate-thirdwave closed 3 months ago

nate-thirdwave commented 4 months ago

The no_sanitize_address nerf for crc64 also needs no_sanitize_thread since the tsan checker also looks for heap-use-after-free.

--- src/liblzma/check/crc64_fast.c  2023-11-01 12:19:29.000000000 +0000
+++ src/liblzma/check/crc64_fast.c-new  2024-05-30 16:56:41.935335535 +0000
@@ -209,11 +209,14 @@
 // The intrinsics use 16-byte-aligned reads from buf, thus they may read
 // up to 15 bytes before or after the buffer (depending on the alignment
 // of the buf argument). The values of the extra bytes are ignored.
-// This unavoidably trips -fsanitize=address so address sanitizier has
-// to be disabled for this function.
+// This unavoidably trips -fsanitize=address and -fsanitize=thread
+// so the sanitizers have to be disabled for this function.
 #if lzma_has_attribute(__no_sanitize_address__)
 __attribute__((__no_sanitize_address__))
 #endif
+#if lzma_has_attribute(__no_sanitize_thread__)
+__attribute__((__no_sanitize_thread__))
+#endif
 static uint64_t
 crc64_clmul(const uint8_t *buf, size_t size, uint64_t crc)
 {
nate-thirdwave commented 4 months ago

Observing a tsan fault depends on the state of your heap (some other previously free'd memory block needs to be in the vestigial space before or after the buffer)

Larhzu commented 4 months ago

Perhaps with Clang it could be __attribute__((__no_sanitize__("*"))) or such as clearly it can trip about every sanitizer. It's funny as in assembly code the code would be fine. But in any case the current code will be replaced with one that won't trip sanitizers and not require these attributes.

Larhzu commented 3 months ago

See #127.