Skip to content

bpo-46561: Ensure operands to __get__ survive the call #30979

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tekknolagi
Copy link
Contributor

@tekknolagi tekknolagi commented Jan 28, 2022

Callees can assume their parameters survive for the entire call. This
violates that assumption and can cause a use-after-free.

https://bugs.python.org/issue46561

Callees can assume their parameters survive for the entire call. This
violates that assumption and can cause a use-after-free.

This is not an issue in CPython right now because later on in the
interpreter __get__ fastcall path, the whole vector of arguments get
INCREFed. However, if a program provides a different entrypoint for a
vectorcall, it may crash.
@markshannon
Copy link
Member

The changes look good, could you add some test cases?

@tekknolagi
Copy link
Contributor Author

I am working on making a C-API equivalent for your sample Python test code. Unfortunately, it is not so easy as making a C extension class with Py_tp_descr_get because that path appears to do the right thing. So instead I have to make a class which has something (a function or C extension callable) with a tp_vectorcall for __get__.

facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Mar 1, 2022
Summary:
Callees can assume their parameters survive for the entire
call. This violates that assumption and can cause a use-after-free.
Similar to D27254519. See python/cpython#30979.

Reviewed By: swtaarrs

Differential Revision: D33699901

fbshipit-source-id: 677d97d
@tekknolagi
Copy link
Contributor Author

Lol, did this finally bite someone else?

@serhiy-storchaka
Copy link
Member

I am on a mission to review old PRs that were not reviewed by anybody.

LGTM, but please fix the NEWS entry (and its text is not very clear, it could be improved). It would be nice to add tests, but if it is too complicated, it is not necessary.

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 6, 2025

The following commit authors need to sign the Contributor License Agreement:

CLA signed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants