Cyan4973 / FiniteStateEntropy

New generation entropy codecs : Finite State Entropy and Huff0
BSD 2-Clause "Simplified" License
1.33k stars 143 forks source link

Crash when FSE_MAX_MEMORY_USAGE 13 #101

Closed clbr closed 4 years ago

clbr commented 4 years ago
  1. Clone current master
  2. Edit FSE_MAX_MEMORY_USAGE to 13
  3. fse blows up its stack when compressing

Valgrind and gdb traces are useless. Changing DEBUGLEVEL to 1 in debug.h does not help, no assert fires. Compiling with -fstack-protector says " stack smashing detected " and gdb says it came when exiting FSE_compress2 at ../lib/fse_compress.c:706.

clbr commented 4 years ago

Happens with multiple gcc versions.

clbr commented 4 years ago

Seems to still happen when I update fse_compress.c and related files from zstd. (needed some edits)

Cyan4973 commented 4 years ago

Thanks for reporting @clbr .

You mean you reduced FSE_MAX_MEMORY_USAGE from 14 to 13 ? This should have been fine, as long as FSE_DEFAULT_MEMORY_USAGE <= FSE_MAX_MEMORY_USAGE.

Maybe some of the more recent changes impacted this scenario. To be checked.

Cyan4973 commented 4 years ago

I made a quick test, lowering FSE_MAX_MEMORY_USAGE to 13, and got this error : Error 23 : Compression error : workspace buffer is too small.

So, I don't know what's is the exact usage of fse in your case, but I can easily see such error becoming a memory stack access if it is undetected.

Now onto a fix.

Cyan4973 commented 4 years ago

I've uploaded a fix in dev branch that seems to solve the issue (tested with FSE_MAX_MEMORY_USAGE 13 and 12)

clbr commented 4 years ago

My fse usage was simply "./fse xxhash.c". It also triggered when trying to execute the tests, or compressing via calling the library functions. In no case did I get the error, curious.

I confirm the lower values now work, tested 13, 12 and 11. Closing.