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

Limit range of operations on Indexes in 32-bit mode #4129

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Conversation

Cyan4973
Copy link
Contributor

This is a mitigation strategy:
in 32-bit mode, when comparing pointers at very large distances, the result may eventually become negative, since ptrdiff_t is a signed 32-bit value. It happens if the distance is larger than 2 GB, which is technically possible, even if rare.

This patch will reduce the range of possible distances in 32-bit mode to always remain < 2 GB.

Also, in ZSTD_window_update(), the operation is modified to use unsigned type rather than signed, as it seems it could impact the outcome of a following comparison test. I'm unable to tell if this can lead to a bug, or just to a suboptimal decision, but it's suspicious enough that we don't want that to happen.

Ultimately, I would also prefer to rewrite ZSTD_window_update() in a way which doesn't rely on pointer comparisons, and would therefore be less likely to land into UB territory (even if it works). But modifying that part has sprawling consequences across the entire compression code base, hence it's a much larger work, which will require more time.

and use unsigned type.
This reduce risks that an operation produces a negative number when crossing the 2 GB limit.
@Cyan4973 Cyan4973 merged commit 20707e3 into dev Aug 22, 2024
94 checks passed
@Cyan4973 Cyan4973 deleted the mitigate_32bit branch September 3, 2024 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants