Check that new heap types cannot be created uninitialised: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag

[pablogsal]

BPO	43916
Nosy	@vstinner, @tiran, @serhiy-storchaka, @pablogsal, @miss-islington, @erlend-aasland, @shreyanavigyan
PRs	bpo-43916: Add _PyType_DisabledNew to prevent new heap types from being created uninitialised #25653 bpo-43916: Add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag #25721 bpo-43916: Use Py_TPFLAGS_DISABLE_NEW flag in hashlib #25722 [WIP] bpo-43916: PyType_FromSpec() supports Py_tp_new=NULL slot #25733 bpo-43916: Remove _disabled_new() function #25745 bpo-43916: Apply Py_TPFLAGS_DISALLOW_INSTANTIATION to selected types #25748 bpo-43916: PyStdPrinter_Type uses Py_TPFLAGS_DISALLOW_INSTANTIATION #25749 bpo-43916: Select now uses Py_TPFLAGS_DISALLOW_INSTANTIATION #25750 bpo-43916: select.devpoll uses Py_TPFLAGS_DISALLOW_INSTANTIATION #25751 bpo-43916: _md5.md5 uses Py_TPFLAGS_DISALLOW_INSTANTIATION #25753 bpo-43916: Export the _PyStructSequence_InitType to fix build errors in the curses module #25768 bpo-43916: Rewrite new hashlib tests, fix typo (GH-25791) #25791 bpo-43916: Move the _PyStructSequence_InitType function to the internal API #25854 bpo-43916: Use test.support.check_disallow_instantiation in test_tcl #26412 [3.10] bpo-43916: Use test.support.check_disallow_instantiation() in test_tcl (GH-26412) #26888
Files	patch.diff disablednew.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-04-22.21:31:58.958>
labels = ['expert-C-API', 'type-bug', '3.9']
title = 'Check that new heap types cannot be created uninitialised: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag'
updated_at = <Date 2022-01-27.13:51:31.586>
user = 'https://github.com/pablogsal'

bugs.python.org fields:

activity = <Date 2022-01-27.13:51:31.586>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['C API']
creation = <Date 2021-04-22.21:31:58.958>
creator = 'pablogsal'
dependencies = []
files = ['49989', '49992']
hgrepos = []
issue_num = 43916
keywords = ['patch']
message_count = 72.0
messages = ['391634', '391635', '391903', '391905', '391909', '391910', '391911', '391912', '391913', '391917', '391924', '391925', '391926', '391928', '391929', '391931', '391934', '391936', '391937', '391984', '391992', '391999', '392036', '392040', '392042', '392044', '392046', '392048', '392057', '392169', '392170', '392297', '392414', '392426', '392427', '392433', '392434', '392435', '392437', '392438', '392451', '392456', '392462', '392464', '392465', '392472', '392473', '392526', '392528', '392534', '392537', '392545', '392546', '392553', '392554', '392556', '392558', '392570', '392621', '392632', '392635', '392811', '394571', '394582', '394603', '396200', '396476', '411785', '411786', '411865', '411867', '411874']
nosy_count = 7.0
nosy_names = ['vstinner', 'christian.heimes', 'serhiy.storchaka', 'pablogsal', 'miss-islington', 'erlendaasland', 'shreyanavigyan']
pr_nums = ['25653', '25721', '25722', '25733', '25745', '25748', '25749', '25750', '25751', '25753', '25768', '25791', '25854', '26412', '26888']
priority = None
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43916'
versions = ['Python 3.9']

[pablogsal]

I'm creating this issue and marking it as a deferred blocker to make sure we don't forget to check this (adding tests) before the release.

Check this message from Serhiy regarding heap typed concerns:

https://bugs.python.org/msg391598

[pablogsal]

Serhiy's comment for reference:

Static type with tp_new = NULL does not have public constructor, but heap type inherits constructor from base class. As result, it allows to create instances without proper initialization, that can lead to crash. It was fixed for few standard heap types in bpo-23815, then reintroduced, then fixed again in bpo-42694. But it should be checked for every type without constructor.

[serhiy-storchaka]

From Objects/typeobject.c:

 /* The condition below could use some explanation.
 It appears that tp_new is not inherited for static types
 whose base class is 'object'; this seems to be a precaution
 so that old extension types don't suddenly become
 callable (object.__new__ wouldn't insure the invariants
 that the extension type's own factory function ensures).
 Heap types, of course, are under our control, so they do
 inherit tp_new; static extension types that specify some
 other built-in type as the default also
 inherit object.__new__. */
 if (base != &PyBaseObject_Type ||
 (type->tp_flags & Py_TPFLAGS_HEAPTYPE)){
 if (type->tp_new == NULL)
 type->tp_new = base->tp_new;
 }

[serhiy-storchaka]

The explaining comment was added in f884b74.

The code itself is much older. It was originally added in 6d6c1a3, then
weaked in c11e192 and strengthen in 29687cd.

All types except static types which are direct descendants of object inherit tp_new from a base class.

We need to check which static types had tp_new = NULL before they were converted to heap types and set it to NULL manually after type creation.

[erlend-aasland]

We need to check which static types had tp_new = NULL before they were converted to heap types

I did a quick git grep. Results pasted into the list over at https://discuss.python.org/t/list-of-built-in-types-converted-to-heap-types/8403.

[serhiy-storchaka]

It is incomplete. For example arrayiterator did have tp_new = NULL (in other words, PyArrayIter_Type.tp_new was not set to non-NULL value).

In 3.9:

>>> import array
>>> type(iter(array.array('I')))()
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
TypeError: cannot create 'arrayiterator' instances

In 3.10:

>>> import array
>>> type(iter(array.array('I')))()
<array.arrayiterator object at 0x7f02f88db5c0>

[erlend-aasland]

It is incomplete.

Yes, I know. I'm working on completing it manually. Some of the static structs were only partially initialised (most stop at tp_members). The rest of the struct (including tp_new) would then be initialised to zero.

[serhiy-storchaka]

Thank you for your work Erlend.

[pablogsal]

Serhiy, do you think this should be a release blocker for the beta?