Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZSTD_initStaticCDict does not set level and it results in use of uninit value #3525

Closed
danlark1 opened this issue Mar 6, 2023 · 3 comments
Closed
Assignees
Labels

Comments

@danlark1
Copy link
Contributor

danlark1 commented Mar 6, 2023

Describe the bug

ZSTD_initStaticCDict does not set compression level

To Reproduce

While running under msan, we found an error

==9312==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7f4749a338a5 in ZSTD_compressBegin_usingCDict_internal zstd/compress/zstd_compress.c:5503:9
    #1 0x7f4749a339e3 in ZSTD_compress_usingCDict_internal zstd/compress/zstd_compress.c:5548:5
    #2 0x7f4749a339e3 in ZSTD_compress_usingCDict zstd/compress/zstd_compress.c:5574:12
    #3 0x7f474a860012 in basicUnitTests zstd/tests/fuzzer.c:2596:29
    #4 0x7f474a8505b2 in main zstd/tests/fuzzer.c:4734:18

This is the error

ZSTD_CCtxParams_init_internal(&cctxParams, &params, cdict->compressionLevel);

cdict->compressionLevel was not set during initialization

Expected behavior

No msan error

Desktop (please complete the following information):

  • OS: Linux
  • Compiler: clang

Additional context
Should be easy to fix

@yoniko
Copy link
Contributor

yoniko commented Mar 7, 2023

Thank you for reporting @danlark1.

cdict->compressionLevel is indeed not set in all cases which should be fixed.
After experimentation I managed to reproduce by compiling with -O0, so I believe the reason we haven't caught it before is because the compiler is inlining / optimizing things away.

I'll add initialization and verify that we cover this case in CI.

yoniko added a commit to yoniko/zstd that referenced this issue Mar 7, 2023
- Initializes clevel in `ZSTD_CCtxParams_init` (NOTE: currently commented out to verify CI failure, will be amended)
- Adds CI workflow for msan fuzzers runs without optimization (`-O0`)
- Fixes Makefile to correctly pass on user defined `MOREFLAGS` and `FUZZER_FLAGS` in cases they have been overwritten
yoniko added a commit to yoniko/zstd that referenced this issue Mar 7, 2023
- Initializes clevel in `ZSTD_CCtxParams_init`
- Adds CI workflow for msan fuzzers runs without optimization (`-O0`)
- Fixes Makefile to correctly pass on user defined `MOREFLAGS` and `FUZZER_FLAGS` in cases they have been overwritten
yoniko added a commit that referenced this issue Mar 7, 2023
- Initializes clevel in `ZSTD_CCtxParams_init`
- Adds CI workflow for msan fuzzers runs without optimization (`-O0`)
- Fixes Makefile to correctly pass on user defined `MOREFLAGS` and `FUZZER_FLAGS` in cases they have been overwritten
@yoniko
Copy link
Contributor

yoniko commented Mar 7, 2023

Fixed in #3257.
@danlark1 - please verify fix works for you.

@danlark1
Copy link
Contributor Author

danlark1 commented Mar 7, 2023

It works, thanks!

@danlark1 danlark1 closed this as completed Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants