classification
Title: crash in methods of a subclass of _collections.deque with a bad __new__()
Type: crash Stage: resolved
Components: Extension Modules Versions: Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Oren Milman, benjamin.peterson, miss-islington, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-09-27 10:43 by Oren Milman, last changed 2018-09-11 20:42 by benjamin.peterson. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3788 merged Oren Milman, 2017-09-27 11:17
PR 9177 merged miss-islington, 2018-09-11 18:47
PR 9179 merged benjamin.peterson, 2018-09-11 19:01
Messages (7)
msg303125 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-09-27 10:43
The following code causes the interpreter to crash:

import _collections
class BadDeque(_collections.deque):
    def __new__(cls, *args):
        if len(args):
            return 42
        return _collections.deque.__new__(cls)

BadDeque() * 42

(The interpreter would crash also if we replaced 'BadDeque() * 42' with
'BadDeque() + _collections.deque([42])'.)

This is because deque_copy() (in Modules/_collectionsmodule.c) returns whatever
BadDeque() returned, without verifying it is a deque.
deque_repeat() assumes that deque_copy() returned a deque, and passes it to
deque_inplace_repeat(), which assumes it is a deque, and crashes.

(Similarly, deque_concat() assumes that deque_copy() returned a deque, which
is the reason for the other crash.)


ISTM it is a very unlikely corner case, so that adding a test (as well as
a NEWS.d item) for it is unnecessary.
What do you think?
msg303133 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-27 12:58
All other sequence objects return an instance of the base class rather than a subclass. list, tuple, str, bytes, bytearray, array. Only deque tries to create an instance of a subclass.
msg304000 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-09 20:53
I meant that if make deque.copy(), deque.__add__() and deque.__mul__() returning an exact deque for subclasses, this would make the deque class more consistent with other collections and solve this issue. This could even make them (almost) atomic.
msg325049 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2018-09-11 18:46
New changeset 24bd50bdcc97d65130c07d6cd26085fd06c3e972 by Benjamin Peterson (Oren Milman) in branch 'master':
closes bpo-31608: Fix a crash in methods of a subclass of _collections.deque with a bad __new__(). (GH-3788)
https://github.com/python/cpython/commit/24bd50bdcc97d65130c07d6cd26085fd06c3e972
msg325054 - (view) Author: miss-islington (miss-islington) Date: 2018-09-11 19:08
New changeset 536e45accf8f05355dd943a6966b9968cdb15f5a by Miss Islington (bot) in branch '3.7':
closes bpo-31608: Fix a crash in methods of a subclass of _collections.deque with a bad __new__(). (GH-3788)
https://github.com/python/cpython/commit/536e45accf8f05355dd943a6966b9968cdb15f5a
msg325055 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2018-09-11 19:12
New changeset ccbbdd0a1e00ecad6f0005438dd6ff6d84fd9ceb by Benjamin Peterson in branch '3.6':
[3.6] closes bpo-31608: Fix a crash in methods of a subclass of _collections.deque with a bad __new__(). (GH-9178)
https://github.com/python/cpython/commit/ccbbdd0a1e00ecad6f0005438dd6ff6d84fd9ceb
msg325057 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2018-09-11 20:42
New changeset 253279c616d4f34287c5749df15e20eb2eb988d6 by Benjamin Peterson in branch '2.7':
[2.7] closes bpo-31608: Fix a crash in methods of a subclass of _collections.deque with a bad __new__(). (GH-9179)
https://github.com/python/cpython/commit/253279c616d4f34287c5749df15e20eb2eb988d6
History
Date User Action Args
2018-09-11 20:42:01benjamin.petersonsetmessages: + msg325057
2018-09-11 19:12:46benjamin.petersonsetmessages: + msg325055
2018-09-11 19:08:15miss-islingtonsetnosy: + miss-islington
messages: + msg325054
2018-09-11 19:01:38benjamin.petersonsetpull_requests: + pull_request8618
2018-09-11 18:47:08miss-islingtonsetpull_requests: + pull_request8616
2018-09-11 18:46:58benjamin.petersonsetstatus: open -> closed

nosy: + benjamin.peterson
messages: + msg325049

resolution: fixed
stage: patch review -> resolved
2017-10-09 20:53:19serhiy.storchakasetmessages: + msg304000
2017-09-27 12:58:18serhiy.storchakasetversions: + Python 2.7, Python 3.6
nosy: + serhiy.storchaka, rhettinger

messages: + msg303133

assignee: rhettinger
2017-09-27 11:17:49Oren Milmansetkeywords: + patch
stage: patch review
pull_requests: + pull_request3774
2017-09-27 10:43:58Oren Milmancreate