Skip to content

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

Closed

Description

@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']

    Activity

    1. pablogsal commented on Apr 22, 2021

      @pablogsal
      MemberAuthor

      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

    2. pablogsal commented on Apr 22, 2021

      @pablogsal
      MemberAuthor

      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.

    3. serhiy-storchaka commented on Apr 26, 2021

      @serhiy-storchaka
      Member

      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;
          }
      
    4. serhiy-storchaka commented on Apr 26, 2021

      @serhiy-storchaka
      Member

      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.

    5. erlend-aasland commented on Apr 26, 2021

      @erlend-aasland
      Contributor

      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.

    6. serhiy-storchaka commented on Apr 26, 2021

      @serhiy-storchaka
      Member

      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>
    7. erlend-aasland commented on Apr 26, 2021

      @erlend-aasland
      Contributor

      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.

    8. serhiy-storchaka commented on Apr 26, 2021

      @serhiy-storchaka
      Member

      Thank you for your work Erlend.

    9. pablogsal commented on Apr 26, 2021

      @pablogsal
      MemberAuthor

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

    10. 72 remaining items

    11. erlend-aasland commented on Jan 27, 2022

      @erlend-aasland
      Contributor

      Can we close this issue? Or is there a remaining task?

      3.9 is still affected; we should fix those types first.

    12. changed the title [-]Mark static types newly converted to heap types as immutable: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag[/-] [+]Check that new heap types cannot be created uninitialised: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag[/+] on Jan 27, 2022
    13. changed the title [-]Mark static types newly converted to heap types as immutable: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag[/-] [+]Check that new heap types cannot be created uninitialised: add Py_TPFLAGS_DISALLOW_INSTANTIATION type flag[/+] on Jan 27, 2022
    14. erlend-aasland commented on Jan 27, 2022

      @erlend-aasland
      Contributor

      FYI: There are only two bug-fix releases left for 3.9.

    15. vstinner commented on Jan 27, 2022

      @vstinner
      Member

      3.9 is still affected; we should fix those types first.

      I'm against backporting the new type flag, but we can try to set explicitly tp_set to NULL *after* calling PyType_Ready().

    16. transferred this issue fromon Apr 10, 2022
    17. erlend-aasland commented on May 18, 2022

      @erlend-aasland
      Contributor

      Victor:

      Can we close this issue? Or is there a remaining task?

      3.9 is still affected; we should fix those types first.

      3.9 has now entered security-only phase, so I guess that ship has sailed. Suggesting to close this.

    18. added
      pendingThe issue will be closed if no feedback is provided
      on May 18, 2022
    19. vstinner commented on May 18, 2022

      @vstinner
      Member

      Right, I closed the issue.

    20. removed
      pendingThe issue will be closed if no feedback is provided
      on May 19, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions