Fix segfault if pthread_getattr_np fails #76521
Merged
Conversation
This file contains 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
glibc destroys[1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @LukasKalbertodt (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
Looking at documentation and implementation of pthread_getattr_np in glibc, musl, and NetBSD, the behaviour seems consistent. The function is responsible for attr initialization, so that pthread_attr_init is unnecessary, and attributes should be only destroyed on success. On the other hand the pthread_attr_get_np expects already initialized attributes (FreeBSD man page contains somewhat unusual comment: "It is HIGHLY RECOMMENDED to use pthread_attr_init(3) function to allocate attribute storage."). |
|
Ah that makes sense, I'll refactor the code to do that. I initially thought this was just a glibc oddity since musl doesn't call |
…ibcs The calling convention of pthread_getattr_np() is to initialize the pthread_attr_t, so _destroy() is only necessary on success (and _init() isn't necessary beforehand). On the other hand, FreeBSD wants the attr_t to be initialized before pthread_attr_get_np(), and therefore it should always be destroyed afterwards.
|
I'm not qualified to review this PR. @rust-lang/libs can anyone of you take this? Sorry :< |
|
@bors r+ |
|
📌 Commit a684153 has been approved by |
bors
added
S-waiting-on-bors
RalfJung
added a commit
to RalfJung/rust
that referenced
this pull request
Sep 19, 2020
…roy, r=Amanieu Fix segfault if pthread_getattr_np fails glibc [destroys][1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 19, 2020
…roy, r=Amanieu Fix segfault if pthread_getattr_np fails glibc [destroys][1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 19, 2020
…roy, r=Amanieu Fix segfault if pthread_getattr_np fails glibc [destroys][1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 19, 2020
…roy, r=Amanieu Fix segfault if pthread_getattr_np fails glibc [destroys][1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
RalfJung
added a commit
to RalfJung/rust
that referenced
this pull request
Sep 20, 2020
…roy, r=Amanieu Fix segfault if pthread_getattr_np fails glibc [destroys][1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 21, 2020
…roy, r=Amanieu Fix segfault if pthread_getattr_np fails glibc [destroys][1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 21, 2020
…roy, r=Amanieu Fix segfault if pthread_getattr_np fails glibc [destroys][1] the passed pthread_attr_t if pthread_getattr_np() fails. Destroying it again leads to a segfault. Fix it by only destroying it on success for glibc. [1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_getattr_np.c;h=ce437205e41dc05653e435f6188768cccdd91c99;hb=HEAD#l205
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 21, 2020
Rollup of 10 pull requests Successful merges: - rust-lang#76439 (Add error explanation for E0755) - rust-lang#76521 (Fix segfault if pthread_getattr_np fails) - rust-lang#76835 (make replace_prefix only take &str as arguments ) - rust-lang#76967 (Revert adding Atomic::from_mut.) - rust-lang#76977 (Add a regression test for copy propagation miscompilation) - rust-lang#76981 (liballoc bench use imported path Bencher) - rust-lang#76983 (BTreeMap: extra testing & fixed comments) - rust-lang#76996 (Fix typo in rustc_lexer docs) - rust-lang#77009 (Dogfood total_cmp in the test crate) - rust-lang#77012 (update Miri for another bugfix) Failed merges: - rust-lang#76489 (Add explanation for E0756) r? `@ghost`
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.
glibc destroys the passed pthread_attr_t if pthread_getattr_np()
fails. Destroying it again leads to a segfault. Fix it by only
destroying it on success for glibc.