bpo-37012: Fix a possible crash due to PyType_FromSpecWithBases()

[ZackerySpytz]

If the PyObject_MALLOC() call failed in PyType_FromSpecWithBases(),
PyObject_Free() would be called on a static string in type_dealloc().

https://bugs.python.org/issue37012

[andresdelfino]

@ZackerySpytz the "skip issue" label (and any other label for that matter) can only be added by developers. I was also confused when I started doing PRs, and tried adding "skip issue" in several places myself :) I mention this just to let you know why it didn't work.

[Mariatta]

I think this might require issue and news entry.

[serhiy-storchaka]

In what circumstances it is not NULL? PyType_GenericAlloc() fills the type object with zeros.

[vstinner]

I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches

[encukou]

I believe Serhyi's question is valid. If you have an answer, please re-open the PR.

(Is this silencing some static analyzer? )

[ZackerySpytz]

@encukou Please reopen this PR.

*(void**)(res_start + slotoffsets[slot->slot]) = slot->pfunc; sets type->tp_doc to a pointer to a static string. A copy of the docstring is then made, but if the PyObject_MALLOC() call fails, type->tp_doc still points to the static string (e.g. SSLError_doc) when the type is decrefed.

[encukou]

My apologies, you're right.

[miss-islington]

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

[bedevere-bot]

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

[miss-islington]

Sorry, @ZackerySpytz and @encukou, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 0613c1e481440aa8f54ba7f6056924c175fbcc13 2.7

[vstinner]

Instead of reverting the change that we just made, what about not writing into tp_doc if it's value makes the type inconsistent? What about this patch:

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index b28f494962..59d8a9d15f 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -2986,7 +2986,6 @@ PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases)
         if (slot->slot == Py_tp_base || slot->slot == Py_tp_bases)
             /* Processed above */
             continue;
-        *(void**)(res_start + slotoffsets[slot->slot]) = slot->pfunc;
 
         /* need to make a copy of the docstring slot, which usually
            points to a static string literal */
@@ -2995,13 +2994,15 @@ PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases)
             size_t len = strlen(old_doc)+1;
             char *tp_doc = PyObject_MALLOC(len);
             if (tp_doc == NULL) {
-                type->tp_doc = NULL;
                 PyErr_NoMemory();
                 goto fail;
             }
             memcpy(tp_doc, old_doc, len);
             type->tp_doc = tp_doc;
         }
+        else {
+            *(void**)(res_start + slotoffsets[slot->slot]) = slot->pfunc;
+        }
 
         /* Move the slots to the heap type itself */
         if (slot->slot == Py_tp_members) {