Skip to content

gh-127667: fix more reference leaks in hashlib #127668

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Mar 3, 2025
Merged

gh-127667: fix more reference leaks in hashlib #127668

merged 18 commits into from
Mar 3, 2025

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Dec 6, 2024 •
edited
Loading

I took the liberty of changing how exceptions are being set in hashlib because _setException(PyExc_ValueError, NULL); is honestly confusing (namely) that "NULL" stands for a default message when the OpenSSL error message cannot be retrieved). (this will be in a follow-up PR)

In this PR we:

  • release allocated resources in error-branches where they were previously leaking;
  • consistently collapse _setException() + return into one statement when possible; and
  • suppress unused return values for _setException().

@bedevere-app bedevere-app bot mentioned this pull request Dec 6, 2024
Closed
@picnixz picnixz added skip news needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Dec 6, 2024
@picnixz picnixz marked this pull request as draft December 6, 2024 09:30
@picnixz picnixz marked this pull request as ready for review December 6, 2024 10:41
@picnixz picnixz requested a review from gpshead December 6, 2024 16:24
@picnixz picnixz added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Dec 6, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 1b7c775 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Dec 6, 2024
@picnixz picnixz added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 6, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 1b7c775 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 6, 2024
@erlend-aasland
Copy link
Contributor

I'd prefer if this could be broken in two PRs: one for the refactoring, and one for the error branch fixes.

@picnixz
Copy link
Member Author

picnixz commented Jan 4, 2025

I'd prefer if this could be broken in two PRs: one for the refactoring, and one for the error branch fixes.

Sure! I'll split them into two tomorrow.

@picnixz picnixz marked this pull request as draft January 5, 2025 08:35
@picnixz picnixz force-pushed the fix/hashlib/error-branches-127667 branch from f2a0f6f to 9e34c0e Compare January 5, 2025 08:44
@picnixz picnixz marked this pull request as ready for review January 5, 2025 08:47
@picnixz picnixz marked this pull request as draft March 2, 2025 12:48
@picnixz picnixz marked this pull request as ready for review March 2, 2025 12:52
@picnixz picnixz requested a review from tiran as a code owner March 2, 2025 12:52
@picnixz picnixz requested a review from encukou March 2, 2025 12:52
@picnixz picnixz changed the title gh-127667: improve hashlib error-branches gh-127667: fix more reference leaks in hashlib Mar 2, 2025
@picnixz picnixz self-assigned this Mar 2, 2025
@picnixz picnixz merged commit 0978465 into python:main Mar 3, 2025
47 checks passed
@miss-islington-app
Copy link

Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

@picnixz picnixz deleted the fix/hashlib/error-branches-127667 branch March 3, 2025 08:20
@miss-islington-app
Copy link

Sorry, @picnixz, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 097846502b7f33cb327d512e2a396acf4f4de46e 3.13

@miss-islington-app
Copy link

Sorry, @picnixz, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 097846502b7f33cb327d512e2a396acf4f4de46e 3.12

@picnixz
Copy link
Member Author

picnixz commented Mar 3, 2025

Arf, this one won't backport cleanly because it contains some references to the UBSan failures fixes.

picnixz added a commit to picnixz/cpython that referenced this pull request Mar 3, 2025
@picnixz
pythongh-127667: fix memory leaks in hashlib (python#127668)
246c2e2 
- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.
@bedevere-app
Copy link

bedevere-app bot commented Mar 3, 2025

GH-130783 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Mar 3, 2025
@bedevere-app
Copy link

bedevere-app bot commented Mar 3, 2025

GH-130783 is a backport of this pull request to the 3.12 branch.

1 similar comment
@bedevere-app
Copy link

bedevere-app bot commented Mar 3, 2025

GH-130783 is a backport of this pull request to the 3.12 branch.

picnixz added a commit to picnixz/cpython that referenced this pull request Mar 3, 2025
@picnixz
pythongh-127667: fix memory leaks in hashlib (python#127668)
107fda4 
- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.
@bedevere-app
Copy link

bedevere-app bot commented Mar 3, 2025

GH-130784 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Mar 3, 2025
picnixz added a commit that referenced this pull request Mar 3, 2025
@picnixz
[3.12] gh-127667: fix memory leaks in hashlib (GH-127668) (#130783)
d25da89 
gh-127667: fix memory leaks in `hashlib` (GH-127668)

- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.

(cherry-picked from commit 0978465)
picnixz added a commit that referenced this pull request Mar 3, 2025
@picnixz
[3.13] gh-127667: fix memory leaks in hashlib (GH-127668) (#130784)
9643ce9 
gh-127667: fix memory leaks in `hashlib` (GH-127668)

- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.

(cherry-picked from commit 0978465)
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
@picnixz @seehwan80
pythongh-127667: fix memory leaks in hashlib (python#127668)
f133cf0 
- Correctly handle `NULL` values returned by `EVP_MD_CTX_md`.
- Correctly free resources in error branches.
- Consistently suppress `_setException()` return value when needed.
- Collapse `_setException() + return NULL` into a single statement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpshead gpshead gpshead approved these changes

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland

@tiran tiran Awaiting requested review from tiran

@encukou encukou Awaiting requested review from encukou

Assignees

@picnixz picnixz

Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@picnixz @bedevere-bot @erlend-aasland @gpshead