gh-144128: Fix crash in array.fromlist with reentrant __index__#144138
gh-144128: Fix crash in array.fromlist with reentrant __index__#144138vstinner merged 10 commits intopython:mainfrom
Conversation
|
@serhiy-storchaka I have proposed a fix to make array.fromlist() safe against reentrant |
| if (!PyLong_Check(v)) { | ||
| PyObject *orig_v = v; | ||
| Py_INCREF(orig_v); | ||
| v = _PyNumber_Index(v); |
There was a problem hiding this comment.
I think it would make sense to also pass orig_v here. It's technically unnecessary, but it makes the code easier to follow.
There was a problem hiding this comment.
You are right, thanks for pointing it out, from next time I will try to take care of all these stuffs.
…akwY06.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM. I just left minor coding style suggestions.
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
|
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. |
|
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. |
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? |
…python#144138) Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
4 participants
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.