Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34k
gh-144128: Fix crash in array.fromlist with reentrant __index__#144138
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
gh-144128: Fix crash in array.fromlist with reentrant __index__#144138
Uh oh!
There was an error while loading.Please reload this page.
Conversation
priyanshu2282-cyber commentedJan 22, 2026
@serhiy-storchaka I have proposed a fix to make array.fromlist() safe against reentrant |
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Security/2026-01-22-10-18-17.gh-issue-144128.akwY06.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Modules/arraymodule.c Outdated
| if (!PyLong_Check(v)) { | ||
| PyObject *orig_v = v; | ||
| Py_INCREF(orig_v); | ||
| v = _PyNumber_Index(v); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think it would make sense to also passorig_v here. It's technically unnecessary, but it makes the code easier to follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You are right, thanks for pointing it out, from next time I will try to take care of all these stuffs.
Uh oh!
There was an error while loading.Please reload this page.
…akwY06.rstCo-authored-by: Peter Bierma <zintensitydev@gmail.com>
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM. I just left minor coding style suggestions.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
priyanshu2282-cyber commentedJan 23, 2026
Thanks@vstinner for constant suggestions and changes, your feedback is much appriciated and very helpful for me, I will stick to the format and try not to repeat same mistakes in future. |
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM
decb25e intopython:mainUh oh!
There was an error while loading.Please reload this page.
serhiy-storchaka commentedJan 26, 2026
I am not sure this is the right way to fix this issue. Instead of changing some setitems (what about others?), would not it be better to handle this in There was also other similar array issue. |
vstinner commentedJan 26, 2026
I checked the other setters and they don't seem to be vulnerable to the borrowed reference issue. They cannot call arbitrary Python code while using the borrowed reference.
I'm not sure that it would better, it would be basically the same, but requires to modify more code, no?
Would you mind to elaborate? |
Uh oh!
There was an error while loading.Please reload this page.
This change fixes a crash in
array.fromlist()that can happen if an element’s__index__method mutates the input list while it is being processed.Previously,
array.fromlist()assumed the list would remain unchanged during conversion. If__index__cleared the list, the element being converted could be freed while still in use, leading to a crash. The implementation now keeps the element alive for the duration of the conversion, I have added a regression test to cover this case.