◐ Shell
reader mode source ↗
Skip to content

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

Merged
gpshead merged 3 commits into
python:masterfrom
gpshead:use_crypt_r
Dec 30, 2018
Merged

bpo-28503: Use crypt_r() when available instead of crypt()#11373
gpshead merged 3 commits into
python:masterfrom
gpshead:use_crypt_r

Conversation

@gpshead

@gpshead gpshead commented Dec 30, 2018

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor

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

@bedevere-bot

Copy link
Copy Markdown

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
Copy Markdown
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

gpshead commented Nov 15, 2019

Copy link
Copy Markdown
Member Author

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