Skip to content

Conversation

@corona10
Copy link
Member

@corona10corona10 commented Dec 29, 2019

Based on LCatro's report

https://bugs.python.org/issue38588

@corona10corona10 changed the title bpo-38588: Fix segfaults when dict comparision with modifying operandbpo-38588: Fix segfaults when dict comparison with modifying operandDec 29, 2019
@pablogsal
Copy link
Member

Could you include the rest of the cases that LCatro was mentioning? He mentioned another 2 for lists.

@corona10
Copy link
MemberAuthor

corona10 commented Dec 30, 2019

@pablogsal

poc3 is not reproducible on my local mac machine and Linux machine with the master branch.
Looks like the master branch code was changed

cmp=PyObject_RichCompareBool(PyList_GET_ITEM(a, i), el, Py_EQ);

However poc2 is reproducible, so I will add the patch on this PR.

$ cat poc3.py class poc() : def __eq__(self,other) :list1.clear() return NotImplemented list1 = [ set() ] poc() in list1 print('end') $ ./python poc3.py end

@ZackerySpytz
Copy link
Contributor

FWIW, LCatro had opened another issue (https://bugs.python.org/issue38610) for similar crashes in the index(), count(), and remove() methods of list. I had created GH-17022 to address them.

@corona10
Copy link
MemberAuthor

corona10 commented Dec 30, 2019

@pablogsal
Please take a look,
I 've added the list case patch except which is not reproducible in the master branch.
(I've attached the script above)

And for the list_richcompare I've added more cases that were not reported by LCatro.
Both cases are added in the unit test and fixed.

And IMHO, we can close bpo-38588 with this patch
:)

@corona10corona10 changed the title bpo-38588: Fix segfaults when dict comparison with modifying operandbpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBoolDec 30, 2019
@corona10
Copy link
MemberAuthor

@pablogsal Updated!

Copy link
Member

@pablogsalpablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the quick fix, @corona10!

@pablogsalpablogsal merged commit 2d5bf56 into python:masterDec 31, 2019
@miss-islington
Copy link
Contributor

Thanks @corona10 for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

@miss-islington
Copy link
Contributor

Sorry, @corona10 and @pablogsal, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 2d5bf568eaa5059402ccce9ba5a366986ba27c8a 3.7

@pablogsal
Copy link
Member

pablogsal commented Dec 31, 2019

@corona10 Can you make the backport to 3.7 and 3.8 using cherry_picker?

@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

Sorry @corona10 and @pablogsal, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 2d5bf568eaa5059402ccce9ba5a366986ba27c8a 3.8

@bedevere-bot
Copy link

GH-17764 is a backport of this pull request to the 3.8 branch.

corona10 added a commit to corona10/cpython that referenced this pull request Dec 31, 2019
…yObject_RichCompareBool (pythonGH-17734) Take strong references before calling PyObject_RichCompareBool to protect against the case where the object dies during the call. (cherry picked from commit 2d5bf56) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@methane
Copy link
Member

Oh, please check the b.p.o before merge...

@pablogsal
Copy link
Member

Oh, please check the b.p.o before merge...

Apologies, I had totally missed the comment in bpo :(

@bedevere-bot
Copy link

GH-17765 is a backport of this pull request to the 3.7 branch.

@pablogsal
Copy link
Member

Sorry @corona10, I will leave the backports open until we decide what to do finally in the bpo issue.

@corona10
Copy link
MemberAuthor

@pablogsal cc @methane
Yes, that should be done first :)

pablogsal pushed a commit that referenced this pull request Dec 31, 2019
GH-17765) * [3.7] bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool (GH-17734) Take strong references before calling PyObject_RichCompareBool to protect against the case where the object dies during the call.. (cherry picked from commit 2d5bf56) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com> * methane's suggestion methane's suggestion Co-Authored-By: Inada Naoki <songofacandy@gmail.com> Co-authored-by: Inada Naoki <songofacandy@gmail.com>
pablogsal pushed a commit that referenced this pull request Dec 31, 2019
GH-17764) * [3.8] bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool (GH-17734) Take strong references before calling PyObject_RichCompareBool to protect against the case where the object dies during the call. (cherry picked from commit 2d5bf56) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com> * Update Objects/listobject.c @methane's suggestion Co-Authored-By: Inada Naoki <songofacandy@gmail.com> Co-authored-by: Inada Naoki <songofacandy@gmail.com>
@corona10corona10 deleted the bpo-38588 branch December 31, 2019 04:53
corona10 added a commit to corona10/cpython that referenced this pull request Jan 27, 2020
pythonGH-17765) * [3.7] bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool (pythonGH-17734) Take strong references before calling PyObject_RichCompareBool to protect against the case where the object dies during the call.. (cherry picked from commit 2d5bf56) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com> * methane's suggestion methane's suggestion Co-Authored-By: Inada Naoki <songofacandy@gmail.com> Co-authored-by: Inada Naoki <songofacandy@gmail.com>
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
…t_RichCompareBool (pythonGH-17734) Take strong references before calling PyObject_RichCompareBool to protect against the case where the object dies during the call.
Sign up for freeto 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.

8 participants

@corona10@pablogsal@ZackerySpytz@miss-islington@bedevere-bot@methane@rhettinger@the-knights-who-say-ni