Skip to content

gh-134885: zstd: Use Py_XSETREF #134886

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

Merged
merged 2 commits into from
May 30, 2025
Merged

gh-134885: zstd: Use Py_XSETREF #134886

merged 2 commits into from
May 30, 2025

Conversation

JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented May 29, 2025

As the Py_SETREF macro explains, the previous code is theoretically vulnerable to a bug where if the destructor for the previous value of mod_state->CParameter_type accesses this module, it may see a garbage value there.

@JelleZijlstra JelleZijlstra changed the title gh-135885: zstd: Use Py_XSETREF gh-134885: zstd: Use Py_XSETREF May 29, 2025
@@ -1,6 +1,5 @@
import ctypes
import unittest
import warnings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Py_XSETREF() LGTM, but this ctypes correction looks like an accident.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI was broken on main (the import is unused), and since it's a small change I figured I might as well include it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the change is excluded from this PR since f49a07b already applied it, which is good for 3.14.

@encukou encukou merged commit 45c6c48 into python:main May 30, 2025
39 checks passed
@encukou encukou added the needs backport to 3.14 bugs and security fixes label May 30, 2025
@miss-islington-app
Copy link

Thanks @JelleZijlstra for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 30, 2025
(cherry picked from commit 45c6c48)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented May 30, 2025

GH-134922 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 30, 2025
encukou pushed a commit that referenced this pull request May 30, 2025
(cherry picked from commit 45c6c48)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@JelleZijlstra JelleZijlstra deleted the zstd-setref branch May 30, 2025 13:43
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.

3 participants