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

bpo-28503: Use crypt_r() when available instead of crypt() #11373

Merged
merged 3 commits into from
Dec 30, 2018

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented Dec 30, 2018

Use the crypt_r library function instead of crypt when available (currently Linux and FreeBSD) to avoid using static shared libc data structures.

Motivation: This fixes a memory sanitizer failure due to it not understanding libc crypt()'s preallocated space.

We could release the GIL around the crypt_r call, but it is such a fast function anyways that seems rather silly. Do that in a followup PR if desired.

https://bugs.python.org/issue28503

@gpshead gpshead self-assigned this Dec 30, 2018
@gpshead gpshead merged commit 387512c into python:master Dec 30, 2018
@miss-islington
Copy link
Contributor

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-11376 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 30, 2018
…11373)

Use crypt_r() when available instead of crypt() in the crypt module.

As a nice side effect: This also avoids a memory sanitizer flake as clang msan doesn't know about crypt's internal libc allocated buffer.
(cherry picked from commit 387512c)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
@gpshead gpshead deleted the use_crypt_r branch December 30, 2018 23:42
gpshead pushed a commit that referenced this pull request Dec 31, 2018
…GH-11376)

Use crypt_r() when available instead of crypt() in the crypt module.

As a nice side effect: This also avoids a memory sanitizer flake as clang msan doesn't know about crypt's internal libc allocated buffer.
(cherry picked from commit 387512c)

Co-authored-by: Gregory P. Smith <greg@krypto.org> [Google]
csabella pushed a commit to csabella/cpython that referenced this pull request Dec 31, 2018
…11373)

Use crypt_r() when available instead of crypt() in the crypt module.

As a nice side effect: This also avoids a memory sanitizer flake as clang msan doesn't know about crypt's internal libc allocated buffer.
@maxbelanger
Copy link
Contributor

Unless I'm mistaken, this will add -lcrypt (or other) to LIBS, thus linking Python itself to whatever library provides these functions, regardless of whether one decides to ship the _crypt extension. This seems wrong, as only the extension module itself should link against this library, no?

@gpshead
Copy link
Member Author

gpshead commented Nov 15, 2019

if you can demonstrate a problem, feel free to open a new bugs.python.org issue for it.

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.

5 participants