From eb0dd9b6167560b9f911fce79d8ffaf9dcf87dad Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 12 Jun 2023 12:34:11 +0200 Subject: [PATCH 1/2] gh-105375: Explicitly initialise all {Pickler,Unpickler}Object fields PyObject_GC_New() only initialises the object header. All other fields must be explicitly initialised to prevent manipulation of uninitialised fields in dealloc. Align initialisation order with the layout of the object structs. --- Modules/_pickle.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index e6eb9c741e1adc..086c8ddd70cb69 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -1151,21 +1151,25 @@ _Pickler_New(PickleState *st) if (self == NULL) return NULL; + self->memo = NULL; self->pers_func = NULL; + self->pers_func_self = NULL; self->dispatch_table = NULL; - self->buffer_callback = NULL; + self->reducer_override = NULL; self->write = NULL; + self->output_buffer = NULL; + self->output_len = 0; + self->max_output_len = WRITE_BUF_SIZE; self->proto = 0; self->bin = 0; self->framing = 0; self->frame_start = -1; + self->buf_size = 0; self->fast = 0; self->fast_nesting = 0; self->fix_imports = 0; self->fast_memo = NULL; - self->max_output_len = WRITE_BUF_SIZE; - self->output_len = 0; - self->reducer_override = NULL; + self->buffer_callback = NULL; self->memo = PyMemoTable_New(); if (self->memo == NULL) { @@ -1635,7 +1639,13 @@ _Unpickler_New(PyObject *module) if (self == NULL) return NULL; + self->stack = NULL; + self->memo = NULL; + self->memo_size = 32; + self->memo_len = 0; self->pers_func = NULL; + self->pers_func_self = NULL; + memset(&self->buffer, 0, sizeof(Py_buffer)); self->input_buffer = NULL; self->input_line = NULL; self->input_len = 0; @@ -1653,9 +1663,7 @@ _Unpickler_New(PyObject *module) self->marks_size = 0; self->proto = 0; self->fix_imports = 0; - memset(&self->buffer, 0, sizeof(Py_buffer)); - self->memo_size = 32; - self->memo_len = 0; + self->memo = _Unpickler_NewMemo(self->memo_size); if (self->memo == NULL) { Py_DECREF(self); From e4fd63774e79ef77591eb9c0ef06d9252f5b9621 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 12 Jun 2023 13:55:57 +0200 Subject: [PATCH 2/2] Make object init cleaner: - Prepare memory allocations and new objects before calling PyObject_GC_New() - Initialise all fields at once using a series of assignments only - Use error labels for clarity --- Modules/_pickle.c | 82 ++++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 086c8ddd70cb69..9e70fee84e18d3 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -1145,21 +1145,31 @@ _Pickler_Write(PicklerObject *self, const char *s, Py_ssize_t data_len) static PicklerObject * _Pickler_New(PickleState *st) { - PicklerObject *self; - - self = PyObject_GC_New(PicklerObject, st->Pickler_Type); - if (self == NULL) + PyMemoTable *memo = PyMemoTable_New(); + if (memo == NULL) { return NULL; + } - self->memo = NULL; + const Py_ssize_t max_output_len = WRITE_BUF_SIZE; + PyObject *output_buffer = PyBytes_FromStringAndSize(NULL, max_output_len); + if (output_buffer == NULL) { + goto error; + } + + PicklerObject *self = PyObject_GC_New(PicklerObject, st->Pickler_Type); + if (self == NULL) { + goto error; + } + + self->memo = memo; self->pers_func = NULL; self->pers_func_self = NULL; self->dispatch_table = NULL; self->reducer_override = NULL; self->write = NULL; - self->output_buffer = NULL; + self->output_buffer = output_buffer; self->output_len = 0; - self->max_output_len = WRITE_BUF_SIZE; + self->max_output_len = max_output_len; self->proto = 0; self->bin = 0; self->framing = 0; @@ -1171,20 +1181,13 @@ _Pickler_New(PickleState *st) self->fast_memo = NULL; self->buffer_callback = NULL; - self->memo = PyMemoTable_New(); - if (self->memo == NULL) { - Py_DECREF(self); - return NULL; - } - self->output_buffer = PyBytes_FromStringAndSize(NULL, - self->max_output_len); - if (self->output_buffer == NULL) { - Py_DECREF(self); - return NULL; - } - PyObject_GC_Track(self); return self; + +error: + PyMem_Free(memo); + Py_XDECREF(output_buffer); + return NULL; } static int @@ -1632,16 +1635,27 @@ _Unpickler_MemoCleanup(UnpicklerObject *self) static UnpicklerObject * _Unpickler_New(PyObject *module) { - UnpicklerObject *self; + const int MEMO_SIZE = 32; + PyObject **memo = _Unpickler_NewMemo(MEMO_SIZE); + if (memo == NULL) { + return NULL; + } + PickleState *st = _Pickle_GetState(module); + PyObject *stack = Pdata_New(st); + if (stack == NULL) { + goto error; + } - self = PyObject_GC_New(UnpicklerObject, st->Unpickler_Type); - if (self == NULL) - return NULL; + UnpicklerObject *self = PyObject_GC_New(UnpicklerObject, + st->Unpickler_Type); + if (self == NULL) { + goto error; + } - self->stack = NULL; - self->memo = NULL; - self->memo_size = 32; + self->stack = (Pdata *)stack; + self->memo = memo; + self->memo_size = MEMO_SIZE; self->memo_len = 0; self->pers_func = NULL; self->pers_func_self = NULL; @@ -1664,19 +1678,13 @@ _Unpickler_New(PyObject *module) self->proto = 0; self->fix_imports = 0; - self->memo = _Unpickler_NewMemo(self->memo_size); - if (self->memo == NULL) { - Py_DECREF(self); - return NULL; - } - self->stack = (Pdata *)Pdata_New(st); - if (self->stack == NULL) { - Py_DECREF(self); - return NULL; - } - PyObject_GC_Track(self); return self; + +error: + PyMem_Free(memo); + Py_XDECREF(stack); + return NULL; } /* Returns -1 (with an exception set) on failure, 0 on success. This may