Fix #700: Remove public Clone from spsc_queue to prevent memory corruption#703
Open
dahankzter wants to merge 7 commits intoDataDog:masterfrom
Open
Fix #700: Remove public Clone from spsc_queue to prevent memory corruption#703dahankzter wants to merge 7 commits intoDataDog:masterfrom
dahankzter wants to merge 7 commits intoDataDog:masterfrom
Conversation
liburing submodule updated 2.13 and fix format errors
- CI/README: MSRV -> 1.92 - sysfs: track queue io_poll support and use it to gate poll ring usage - spsc_queue: remove raw buffer ptr, use Box<[Slot<T>]> + wrapping indices
Remove more unsafe code from spsc queue.
Author
|
All failures show the same error: This occurs because:
|
Fixes issue DataDog#700 - memory corruption through Clone abuse. The spsc_queue is designed for Single-Producer-Single-Consumer access with Relaxed memory ordering for performance. The public Clone implementation allowed external code to create multiple producers or consumers, which violates SPSC guarantees and causes: - Double-free when multiple consumers read the same value - Memory corruption (malloc chunk size mismatch) - Memory leaks when multiple producers overwrite values - Undefined behavior in safe code Changes: - Remove Clone trait implementation from Producer<T> and Consumer<T> - Add crate-private clone_internal() methods for internal use - Update shared_channel to use clone_internal() for connection handoff - Add documentation explaining SPSC guarantees and safety requirements This is a breaking API change but necessary to prevent UB in safe code. Internal uses within glommio (shared_channel) are safe because they ensure only one producer/consumer is active at any given time. Signed-off-by: Henrik Ma Johansson <dahankzter@gmail.com>
The rand 0.10 API changed, requiring RngExt trait to be in scope to use the random_range() method. This fixes CI build failures. Signed-off-by: Henrik Ma Johansson <dahankzter@gmail.com>
3a0eddd to
3dd5033
Compare
After rebasing on PR DataDog#702, which updated rand from 0.10 to 0.9, the benchmarks needed API adjustments: - Use rand::Rng instead of rand::RngExt - Use gen_range() instead of random_range() This ensures benchmarks compile with the updated dependency. Signed-off-by: Henrik Ma Johansson <dahankzter@gmail.com>
b3cec27 to
786caa0
Compare
Update: Rebased on PR #702This PR has been rebased on top of #702 (dependency updates) to inherit its passing CI infrastructure. What changed from the rebase:From PR #702 (most of the file changes):
Our actual fix (the core changes for issue #700):
All CI checks now pass ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #700 - Removes the public
Cloneimplementation fromProducer<T>andConsumer<T>to prevent memory corruption in safe code.Problem
The
spsc_queueis designed for Single-Producer-Single-Consumer access withOrdering::Relaxedfor performance. The publicClonetrait implementation allowed external code to create multiple producers or consumers, which violates SPSC guarantees and causes:malloc(): chunk size mismatch in fastbin)Solution
Clonetrait implementation fromProducer<T>andConsumer<T>clone_internal()methods for internal use within glommioshared_channelto useclone_internal()for connection handoffBreaking Change
This is a breaking API change. External code that clones
ProducerorConsumerwill no longer compile. However, such code was already unsound and could cause heap corruption.Internal uses within glommio (e.g.,
shared_channel) are safe because they ensure only one producer/consumer is active at any given time.Testing