From: Serhiy Storchaka Date: Sun, 11 Jan 2026 12:01:13 +0000 (+0200) Subject: gh-143638: Forbid cuncurrent use of the Pickler and Unpickler objects in C implementa... X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d1282efb2b847bf9274d78c5f15ea00499b2c894;p=thirdparty%2FPython%2Fcpython.git gh-143638: Forbid cuncurrent use of the Pickler and Unpickler objects in C implementation (GH-143664) Previously, this could cause crash or data corruption, now concurrent calls of methods of the same object raise RuntimeError. --- diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 22c70327fb06..48375cf459ea 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -419,6 +419,46 @@ if has_c_implementation: unpickler.memo = {-1: None} unpickler.memo = {1: None} + def test_concurrent_pickler_dump(self): + f = io.BytesIO() + pickler = self.pickler_class(f) + class X: + def __reduce__(slf): + self.assertRaises(RuntimeError, pickler.dump, 42) + return list, () + pickler.dump(X()) # should not crash + self.assertEqual(pickle.loads(f.getvalue()), []) + + def test_concurrent_pickler_dump_and_init(self): + f = io.BytesIO() + pickler = self.pickler_class(f) + class X: + def __reduce__(slf): + self.assertRaises(RuntimeError, pickler.__init__, f) + return list, () + pickler.dump([X()]) # should not fail + self.assertEqual(pickle.loads(f.getvalue()), [[]]) + + def test_concurrent_unpickler_load(self): + global reducer + def reducer(): + self.assertRaises(RuntimeError, unpickler.load) + return 42 + f = io.BytesIO(b'(c%b\nreducer\n(tRl.' % (__name__.encode(),)) + unpickler = self.unpickler_class(f) + unpickled = unpickler.load() # should not fail + self.assertEqual(unpickled, [42]) + + def test_concurrent_unpickler_load_and_init(self): + global reducer + def reducer(): + self.assertRaises(RuntimeError, unpickler.__init__, f) + return 42 + f = io.BytesIO(b'(c%b\nreducer\n(tRl.' % (__name__.encode(),)) + unpickler = self.unpickler_class(f) + unpickled = unpickler.load() # should not crash + self.assertEqual(unpickled, [42]) + class CDispatchTableTests(AbstractDispatchTableTests, unittest.TestCase): pickler_class = pickle.Pickler def get_dispatch_table(self): @@ -467,7 +507,7 @@ if has_c_implementation: check_sizeof = support.check_sizeof def test_pickler(self): - basesize = support.calcobjsize('7P2n3i2n3i2P') + basesize = support.calcobjsize('7P2n3i2n4i2P') p = _pickle.Pickler(io.BytesIO()) self.assertEqual(object.__sizeof__(p), basesize) MT_size = struct.calcsize('3nP0n') @@ -484,7 +524,7 @@ if has_c_implementation: 0) # Write buffer is cleared after every dump(). def test_unpickler(self): - basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P3n8P2n2i') + basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P3n8P2n3i') unpickler = _pickle.Unpickler P = struct.calcsize('P') # Size of memo table entry. n = struct.calcsize('n') # Size of mark table entry. diff --git a/Misc/NEWS.d/next/Library/2026-01-10-16-42-47.gh-issue-143638.du5G7d.rst b/Misc/NEWS.d/next/Library/2026-01-10-16-42-47.gh-issue-143638.du5G7d.rst new file mode 100644 index 000000000000..fd71db9c8e07 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-01-10-16-42-47.gh-issue-143638.du5G7d.rst @@ -0,0 +1,4 @@ +Forbid reentrant calls of the :class:`pickle.Pickler` and +:class:`pickle.Unpickler` methods for the C implementation. Previously, this +could cause crash or data corruption, now concurrent calls of methods of the +same object raise :exc:`RuntimeError`. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index d4a610e7909b..063547c9a4d0 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -641,6 +641,7 @@ typedef struct PicklerObject { int fast_nesting; int fix_imports; /* Indicate whether Pickler should fix the name of globals for Python 2.x. */ + int running; /* True when a method of Pickler is executing. */ PyObject *fast_memo; PyObject *buffer_callback; /* Callback for out-of-band buffers, or NULL */ } PicklerObject; @@ -685,6 +686,8 @@ typedef struct UnpicklerObject { int proto; /* Protocol of the pickle loaded. */ int fix_imports; /* Indicate whether Unpickler should fix the name of globals pickled by Python 2.x. */ + int running; /* True when a method of Unpickler is executing. */ + } UnpicklerObject; typedef struct { @@ -702,6 +705,32 @@ typedef struct { #define PicklerMemoProxyObject_CAST(op) ((PicklerMemoProxyObject *)(op)) #define UnpicklerMemoProxyObject_CAST(op) ((UnpicklerMemoProxyObject *)(op)) +#define BEGIN_USING_PICKLER(SELF, RET) do { \ + if ((SELF)->running) { \ + PyErr_SetString(PyExc_RuntimeError, \ + "Pickler object is already used"); \ + return (RET); \ + } \ + (SELF)->running = 1; \ + } while (0) + +#define END_USING_PICKLER(SELF) do { \ + (SELF)->running = 0; \ + } while (0) + +#define BEGIN_USING_UNPICKLER(SELF, RET) do { \ + if ((SELF)->running) { \ + PyErr_SetString(PyExc_RuntimeError, \ + "Unpickler object is already used"); \ + return (RET); \ + } \ + (SELF)->running = 1; \ + } while (0) + +#define END_USING_UNPICKLER(SELF) do { \ + (SELF)->running = 0; \ + } while (0) + /* Forward declarations */ static int save(PickleState *state, PicklerObject *, PyObject *, int); static int save_reduce(PickleState *, PicklerObject *, PyObject *, PyObject *); @@ -1131,6 +1160,7 @@ _Pickler_New(PickleState *st) self->fast = 0; self->fast_nesting = 0; self->fix_imports = 0; + self->running = 0; self->fast_memo = NULL; self->buffer_callback = NULL; @@ -1708,6 +1738,7 @@ _Unpickler_New(PyObject *module) self->marks_size = 0; self->proto = 0; self->fix_imports = 0; + self->running = 0; PyObject_GC_Track(self); return self; @@ -4764,17 +4795,23 @@ _pickle_Pickler_dump_impl(PicklerObject *self, PyTypeObject *cls, Py_TYPE(self)->tp_name); return NULL; } + BEGIN_USING_PICKLER(self, NULL); - if (_Pickler_ClearBuffer(self) < 0) - return NULL; - - if (dump(st, self, obj) < 0) - return NULL; - - if (_Pickler_FlushToFile(self) < 0) - return NULL; - + if (_Pickler_ClearBuffer(self) < 0) { + goto error; + } + if (dump(st, self, obj) < 0) { + goto error; + } + if (_Pickler_FlushToFile(self) < 0) { + goto error; + } + END_USING_PICKLER(self); Py_RETURN_NONE; + +error: + END_USING_PICKLER(self); + return NULL; } /*[clinic input] @@ -4915,47 +4952,54 @@ _pickle_Pickler___init___impl(PicklerObject *self, PyObject *file, PyObject *buffer_callback) /*[clinic end generated code: output=0abedc50590d259b input=cddc50f66b770002]*/ { + BEGIN_USING_PICKLER(self, -1); /* In case of multiple __init__() calls, clear previous content. */ if (self->write != NULL) (void)Pickler_clear((PyObject *)self); - if (_Pickler_SetProtocol(self, protocol, fix_imports) < 0) - return -1; - - if (_Pickler_SetOutputStream(self, file) < 0) - return -1; - - if (_Pickler_SetBufferCallback(self, buffer_callback) < 0) - return -1; - + if (_Pickler_SetProtocol(self, protocol, fix_imports) < 0) { + goto error; + } + if (_Pickler_SetOutputStream(self, file) < 0) { + goto error; + } + if (_Pickler_SetBufferCallback(self, buffer_callback) < 0) { + goto error; + } /* memo and output_buffer may have already been created in _Pickler_New */ if (self->memo == NULL) { self->memo = PyMemoTable_New(); - if (self->memo == NULL) - return -1; + if (self->memo == NULL) { + goto error; + } } self->output_len = 0; if (self->output_buffer == NULL) { self->max_output_len = WRITE_BUF_SIZE; self->output_buffer = PyBytes_FromStringAndSize(NULL, self->max_output_len); - if (self->output_buffer == NULL) - return -1; + if (self->output_buffer == NULL) { + goto error; + } } self->fast = 0; self->fast_nesting = 0; self->fast_memo = NULL; - if (self->dispatch_table != NULL) { - return 0; - } - if (PyObject_GetOptionalAttr((PyObject *)self, &_Py_ID(dispatch_table), - &self->dispatch_table) < 0) { - return -1; + if (self->dispatch_table == NULL) { + if (PyObject_GetOptionalAttr((PyObject *)self, &_Py_ID(dispatch_table), + &self->dispatch_table) < 0) { + goto error; + } } + END_USING_PICKLER(self); return 0; + +error: + END_USING_PICKLER(self); + return -1; } @@ -7157,22 +7201,22 @@ static PyObject * _pickle_Unpickler_load_impl(UnpicklerObject *self, PyTypeObject *cls) /*[clinic end generated code: output=cc88168f608e3007 input=f5d2f87e61d5f07f]*/ { - UnpicklerObject *unpickler = (UnpicklerObject*)self; - PickleState *st = _Pickle_GetStateByClass(cls); /* Check whether the Unpickler was initialized correctly. This prevents segfaulting if a subclass overridden __init__ with a function that does not call Unpickler.__init__(). Here, we simply ensure that self->read is not NULL. */ - if (unpickler->read == NULL) { + if (self->read == NULL) { PyErr_Format(st->UnpicklingError, "Unpickler.__init__() was not called by %s.__init__()", - Py_TYPE(unpickler)->tp_name); + Py_TYPE(self)->tp_name); return NULL; } - - return load(st, unpickler); + BEGIN_USING_UNPICKLER(self, NULL); + PyObject *res = load(st, self); + END_USING_UNPICKLER(self); + return res; } /* The name of find_class() is misleading. In newer pickle protocols, this @@ -7436,35 +7480,41 @@ _pickle_Unpickler___init___impl(UnpicklerObject *self, PyObject *file, const char *errors, PyObject *buffers) /*[clinic end generated code: output=09f0192649ea3f85 input=ca4c1faea9553121]*/ { + BEGIN_USING_UNPICKLER(self, -1); /* In case of multiple __init__() calls, clear previous content. */ if (self->read != NULL) (void)Unpickler_clear((PyObject *)self); - if (_Unpickler_SetInputStream(self, file) < 0) - return -1; - - if (_Unpickler_SetInputEncoding(self, encoding, errors) < 0) - return -1; - - if (_Unpickler_SetBuffers(self, buffers) < 0) - return -1; - + if (_Unpickler_SetInputStream(self, file) < 0) { + goto error; + } + if (_Unpickler_SetInputEncoding(self, encoding, errors) < 0) { + goto error; + } + if (_Unpickler_SetBuffers(self, buffers) < 0) { + goto error; + } self->fix_imports = fix_imports; PyTypeObject *tp = Py_TYPE(self); PickleState *state = _Pickle_FindStateByType(tp); self->stack = (Pdata *)Pdata_New(state); - if (self->stack == NULL) - return -1; - + if (self->stack == NULL) { + goto error; + } self->memo_size = 32; self->memo = _Unpickler_NewMemo(self->memo_size); - if (self->memo == NULL) - return -1; - + if (self->memo == NULL) { + goto error; + } self->proto = 0; + END_USING_UNPICKLER(self); return 0; + +error: + END_USING_UNPICKLER(self); + return -1; }