bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool #17734

corona10 · 2019-12-29T10:15:05Z

Based on LCatro's report

https://bugs.python.org/issue38588

pablogsal · 2019-12-29T17:00:30Z

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

corona10 · 2019-12-30T02:07:58Z

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

cpython/Objects/listobject.c

Line 452 in 3c87a66

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 · 2019-12-30T02:22:55Z

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 · 2019-12-30T04:52:32Z

@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
:)

Lib/test/test_list.py

Lib/test/test_dict.py

corona10 · 2019-12-31T00:30:35Z

@pablogsal Updated!

pablogsal

LGTM

Thanks for the quick fix, @corona10!

miss-islington · 2019-12-31T01:04:25Z

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 · 2019-12-31T01:04:26Z

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 · 2019-12-31T01:04:30Z

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 · 2019-12-31T01:04:57Z

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

miss-islington · 2019-12-31T01:05:12Z

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

miss-islington · 2019-12-31T01:05:14Z

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 · 2019-12-31T01:14:33Z

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

…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 <[email protected]>

methane · 2019-12-31T01:15:51Z

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

pablogsal · 2019-12-31T01:17:02Z

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

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

bedevere-bot · 2019-12-31T01:18:10Z

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

pablogsal · 2019-12-31T01:19:26Z

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

corona10 · 2019-12-31T01:30:05Z

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

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 <[email protected]> * methane's suggestion methane's suggestion Co-Authored-By: Inada Naoki <[email protected]> Co-authored-by: Inada Naoki <[email protected]>

@methane

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 <[email protected]> * Update Objects/listobject.c @methane's suggestion Co-Authored-By: Inada Naoki <[email protected]> Co-authored-by: Inada Naoki <[email protected]>

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 <[email protected]> * methane's suggestion methane's suggestion Co-Authored-By: Inada Naoki <[email protected]> Co-authored-by: Inada Naoki <[email protected]>

…t_RichCompareBool (pythonGH-17734) Take strong references before calling PyObject_RichCompareBool to protect against the case where the object dies during the call.

bpo-38588: Fix segfaults when dict comparision with modifying operand

4aaa382

corona10 requested a review from methane as a code owner December 29, 2019 10:15

the-knights-who-say-ni added the CLA signed label Dec 29, 2019

bedevere-bot added the awaiting review label Dec 29, 2019

corona10 changed the title ~~bpo-38588: Fix segfaults when dict comparision with modifying operand~~ bpo-38588: Fix segfaults when dict comparison with modifying operand Dec 29, 2019

rhettinger approved these changes Dec 29, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Dec 29, 2019

bpo-38588: Fix list case

035a9d7

bpo-38588: Add comment

e809f7a

corona10 requested a review from pablogsal December 30, 2019 04:57

corona10 changed the title ~~bpo-38588: Fix segfaults when dict comparison with modifying operand~~ bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool Dec 30, 2019

pablogsal reviewed Dec 30, 2019

View reviewed changes

Lib/test/test_list.py Outdated Show resolved Hide resolved

pablogsal reviewed Dec 30, 2019

View reviewed changes

Lib/test/test_dict.py Outdated Show resolved Hide resolved

bpo-38588: Update comment

645e1fb

pablogsal added needs backport to 3.7 labels Dec 31, 2019

pablogsal approved these changes Dec 31, 2019

View reviewed changes

pablogsal merged commit 2d5bf56 into python:master Dec 31, 2019

bedevere-bot removed the awaiting merge label Dec 31, 2019

miss-islington assigned pablogsal Dec 31, 2019

pablogsal added needs backport to 3.8 and removed needs backport to 3.8 labels Dec 31, 2019

corona10 mentioned this pull request Dec 31, 2019

[3.8] bpo-38588: Fix possible crashes in dict and list when calling P… #17764

Merged

bedevere-bot removed the needs backport to 3.8 label Dec 31, 2019

corona10 mentioned this pull request Dec 31, 2019

[3.7] bpo-38588: Fix possible crashes in dict and list when calling P… #17765

Merged

bedevere-bot removed the needs backport to 3.7 label Dec 31, 2019

corona10 deleted the bpo-38588 branch December 31, 2019 04:53

serhiy-storchaka mentioned this pull request Jun 10, 2024

gh-120298: Fix use-after-free in list_richcompare_impl #120303

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool #17734

bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool #17734

corona10 commented Dec 29, 2019 •

edited by bedevere-bot

Loading

pablogsal commented Dec 29, 2019

corona10 commented Dec 30, 2019 •

edited

Loading

ZackerySpytz commented Dec 30, 2019

corona10 commented Dec 30, 2019 •

edited

Loading

corona10 commented Dec 31, 2019

pablogsal left a comment

miss-islington commented Dec 31, 2019

miss-islington commented Dec 31, 2019

miss-islington commented Dec 31, 2019

pablogsal commented Dec 31, 2019 •

edited

Loading

miss-islington commented Dec 31, 2019

miss-islington commented Dec 31, 2019

bedevere-bot commented Dec 31, 2019

methane commented Dec 31, 2019

pablogsal commented Dec 31, 2019

bedevere-bot commented Dec 31, 2019

pablogsal commented Dec 31, 2019

corona10 commented Dec 31, 2019

bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool #17734

bpo-38588: Fix possible crashes in dict and list when calling PyObject_RichCompareBool #17734

Conversation

corona10 commented Dec 29, 2019 • edited by bedevere-bot Loading

pablogsal commented Dec 29, 2019

corona10 commented Dec 30, 2019 • edited Loading

ZackerySpytz commented Dec 30, 2019

corona10 commented Dec 30, 2019 • edited Loading

corona10 commented Dec 31, 2019

pablogsal left a comment

Choose a reason for hiding this comment

miss-islington commented Dec 31, 2019

miss-islington commented Dec 31, 2019

miss-islington commented Dec 31, 2019

pablogsal commented Dec 31, 2019 • edited Loading

miss-islington commented Dec 31, 2019

miss-islington commented Dec 31, 2019

bedevere-bot commented Dec 31, 2019

methane commented Dec 31, 2019

pablogsal commented Dec 31, 2019

bedevere-bot commented Dec 31, 2019

pablogsal commented Dec 31, 2019

corona10 commented Dec 31, 2019

corona10 commented Dec 29, 2019 •

edited by bedevere-bot

Loading

corona10 commented Dec 30, 2019 •

edited

Loading

corona10 commented Dec 30, 2019 •

edited

Loading

pablogsal commented Dec 31, 2019 •

edited

Loading