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

Fix panic in GroupInfo::members #680

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeremija
Copy link

@jeremija jeremija commented May 5, 2024

I recently observed undefined behavior in one of the projects I'm working on because slice::from_raw_parts is called without a null-check in GroupInfo::members. This undefined behavior was present when iterating over the resulting slice and it would just terminate prematurely when trying to chain multiple iterators. The function is pretty strict about what kind of pointers it accepts:

data must be non-null and aligned even for zero-length slices.

This undefined behavior has become a panic in debug builds in Rust 1.78.0:

For example, slice::from_raw_parts requires an aligned non-null pointer.
The following use of a purposely-misaligned pointer has undefined behavior,
and while if you were unlucky it may have appeared to "work" in the past,
the debug assertion can now catch it:

Cause is found in rdkafka.c. I noticed there are more uses of
slice::from_raw_parts so I replaced all of them except a call to
Vec::from_raw_parts which seems fine. I'd appreciate feedback!

@jeremija jeremija force-pushed the task/add-null-check-in-groupinfo-members branch 3 times, most recently from 531e6d2 to 4470b95 Compare May 6, 2024 08:45
I recently observed undefined behavior in one of the projects I'm
working on because [slice::from_raw_parts] is called without a
null-check in `GroupInfo::members`. This undefined behavior was present
when iterating over the resulting slice and it would just terminate
prematurely when trying to chain multiple iterators. The function is
pretty strict about what kind of pointers it accepts:

> data must be non-null and aligned even for zero-length slices.

This undefined behavior has become a panic in debug builds in [Rust 1.78.0]:

> For example, slice::from_raw_parts requires an aligned non-null pointer.
> The following use of a purposely-misaligned pointer has undefined behavior,
> and while if you were unlucky it may have appeared to "work" in the past,
> the debug assertion can now catch it:

Cause is found in [rdkafka.c]. I see there are more uses of
`slice::from_raw_parts` so I replaced all of them except a call to
`Vec::from_raw_parts` which seems fine. I'd appreciate feedback!

[slice::from_raw_parts]: https://doc.rust-lang.org/std/slice/fn.from_raw_parts.html
[Rust 1.78.0]: https://blog.rust-lang.org/2024/05/02/Rust-1.78.0.html#asserting-unsafe-preconditions
[rdkafka.c]: https://github.com/confluentinc/librdkafka/blob/95a542c87c61d2c45b445f91c73dd5442eb04f3c/src/rdkafka.c#L4668-L4670
@jeremija jeremija force-pushed the task/add-null-check-in-groupinfo-members branch from 4470b95 to 8b7ccac Compare May 6, 2024 08:48
@davidblewett
Copy link
Collaborator

@jeremija can you fix the conflicts reported here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants