Skip to content

Commit d1282ef

Browse files
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.
1 parent 03e6457 commit d1282ef

File tree

3 files changed

+145
-51
lines changed

3 files changed

+145
-51
lines changed

Lib/test/test_pickle.py

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,46 @@ def test_issue18339(self):
419419
unpickler.memo = {-1: None}
420420
unpickler.memo = {1: None}
421421

422+
def test_concurrent_pickler_dump(self):
423+
f = io.BytesIO()
424+
pickler = self.pickler_class(f)
425+
class X:
426+
def __reduce__(slf):
427+
self.assertRaises(RuntimeError, pickler.dump, 42)
428+
return list, ()
429+
pickler.dump(X()) # should not crash
430+
self.assertEqual(pickle.loads(f.getvalue()), [])
431+
432+
def test_concurrent_pickler_dump_and_init(self):
433+
f = io.BytesIO()
434+
pickler = self.pickler_class(f)
435+
class X:
436+
def __reduce__(slf):
437+
self.assertRaises(RuntimeError, pickler.__init__, f)
438+
return list, ()
439+
pickler.dump([X()]) # should not fail
440+
self.assertEqual(pickle.loads(f.getvalue()), [[]])
441+
442+
def test_concurrent_unpickler_load(self):
443+
global reducer
444+
def reducer():
445+
self.assertRaises(RuntimeError, unpickler.load)
446+
return 42
447+
f = io.BytesIO(b'(c%b\nreducer\n(tRl.' % (__name__.encode(),))
448+
unpickler = self.unpickler_class(f)
449+
unpickled = unpickler.load() # should not fail
450+
self.assertEqual(unpickled, [42])
451+
452+
def test_concurrent_unpickler_load_and_init(self):
453+
global reducer
454+
def reducer():
455+
self.assertRaises(RuntimeError, unpickler.__init__, f)
456+
return 42
457+
f = io.BytesIO(b'(c%b\nreducer\n(tRl.' % (__name__.encode(),))
458+
unpickler = self.unpickler_class(f)
459+
unpickled = unpickler.load() # should not crash
460+
self.assertEqual(unpickled, [42])
461+
422462
class CDispatchTableTests(AbstractDispatchTableTests, unittest.TestCase):
423463
pickler_class = pickle.Pickler
424464
def get_dispatch_table(self):
@@ -467,7 +507,7 @@ class SizeofTests(unittest.TestCase):
467507
check_sizeof = support.check_sizeof
468508

469509
def test_pickler(self):
470-
basesize = support.calcobjsize('7P2n3i2n3i2P')
510+
basesize = support.calcobjsize('7P2n3i2n4i2P')
471511
p = _pickle.Pickler(io.BytesIO())
472512
self.assertEqual(object.__sizeof__(p), basesize)
473513
MT_size = struct.calcsize('3nP0n')
@@ -484,7 +524,7 @@ def test_pickler(self):
484524
0) # Write buffer is cleared after every dump().
485525

486526
def test_unpickler(self):
487-
basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P3n8P2n2i')
527+
basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P3n8P2n3i')
488528
unpickler = _pickle.Unpickler
489529
P = struct.calcsize('P') # Size of memo table entry.
490530
n = struct.calcsize('n') # Size of mark table entry.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Forbid reentrant calls of the :class:`pickle.Pickler` and
2+
:class:`pickle.Unpickler` methods for the C implementation. Previously, this
3+
could cause crash or data corruption, now concurrent calls of methods of the
4+
same object raise :exc:`RuntimeError`.

Modules/_pickle.c

Lines changed: 99 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,7 @@ typedef struct PicklerObject {
641641
int fast_nesting;
642642
int fix_imports; /* Indicate whether Pickler should fix
643643
the name of globals for Python 2.x. */
644+
int running; /* True when a method of Pickler is executing. */
644645
PyObject *fast_memo;
645646
PyObject *buffer_callback; /* Callback for out-of-band buffers, or NULL */
646647
} PicklerObject;
@@ -685,6 +686,8 @@ typedef struct UnpicklerObject {
685686
int proto; /* Protocol of the pickle loaded. */
686687
int fix_imports; /* Indicate whether Unpickler should fix
687688
the name of globals pickled by Python 2.x. */
689+
int running; /* True when a method of Unpickler is executing. */
690+
688691
} UnpicklerObject;
689692

690693
typedef struct {
@@ -702,6 +705,32 @@ typedef struct {
702705
#define PicklerMemoProxyObject_CAST(op) ((PicklerMemoProxyObject *)(op))
703706
#define UnpicklerMemoProxyObject_CAST(op) ((UnpicklerMemoProxyObject *)(op))
704707

708+
#define BEGIN_USING_PICKLER(SELF, RET) do { \
709+
if ((SELF)->running) { \
710+
PyErr_SetString(PyExc_RuntimeError, \
711+
"Pickler object is already used"); \
712+
return (RET); \
713+
} \
714+
(SELF)->running = 1; \
715+
} while (0)
716+
717+
#define END_USING_PICKLER(SELF) do { \
718+
(SELF)->running = 0; \
719+
} while (0)
720+
721+
#define BEGIN_USING_UNPICKLER(SELF, RET) do { \
722+
if ((SELF)->running) { \
723+
PyErr_SetString(PyExc_RuntimeError, \
724+
"Unpickler object is already used"); \
725+
return (RET); \
726+
} \
727+
(SELF)->running = 1; \
728+
} while (0)
729+
730+
#define END_USING_UNPICKLER(SELF) do { \
731+
(SELF)->running = 0; \
732+
} while (0)
733+
705734
/* Forward declarations */
706735
static int save(PickleState *state, PicklerObject *, PyObject *, int);
707736
static int save_reduce(PickleState *, PicklerObject *, PyObject *, PyObject *);
@@ -1131,6 +1160,7 @@ _Pickler_New(PickleState *st)
11311160
self->fast = 0;
11321161
self->fast_nesting = 0;
11331162
self->fix_imports = 0;
1163+
self->running = 0;
11341164
self->fast_memo = NULL;
11351165
self->buffer_callback = NULL;
11361166

@@ -1708,6 +1738,7 @@ _Unpickler_New(PyObject *module)
17081738
self->marks_size = 0;
17091739
self->proto = 0;
17101740
self->fix_imports = 0;
1741+
self->running = 0;
17111742

17121743
PyObject_GC_Track(self);
17131744
return self;
@@ -4764,17 +4795,23 @@ _pickle_Pickler_dump_impl(PicklerObject *self, PyTypeObject *cls,
47644795
Py_TYPE(self)->tp_name);
47654796
return NULL;
47664797
}
4798+
BEGIN_USING_PICKLER(self, NULL);
47674799

4768-
if (_Pickler_ClearBuffer(self) < 0)
4769-
return NULL;
4770-
4771-
if (dump(st, self, obj) < 0)
4772-
return NULL;
4773-
4774-
if (_Pickler_FlushToFile(self) < 0)
4775-
return NULL;
4776-
4800+
if (_Pickler_ClearBuffer(self) < 0) {
4801+
goto error;
4802+
}
4803+
if (dump(st, self, obj) < 0) {
4804+
goto error;
4805+
}
4806+
if (_Pickler_FlushToFile(self) < 0) {
4807+
goto error;
4808+
}
4809+
END_USING_PICKLER(self);
47774810
Py_RETURN_NONE;
4811+
4812+
error:
4813+
END_USING_PICKLER(self);
4814+
return NULL;
47784815
}
47794816

47804817
/*[clinic input]
@@ -4915,47 +4952,54 @@ _pickle_Pickler___init___impl(PicklerObject *self, PyObject *file,
49154952
PyObject *buffer_callback)
49164953
/*[clinic end generated code: output=0abedc50590d259b input=cddc50f66b770002]*/
49174954
{
4955+
BEGIN_USING_PICKLER(self, -1);
49184956
/* In case of multiple __init__() calls, clear previous content. */
49194957
if (self->write != NULL)
49204958
(void)Pickler_clear((PyObject *)self);
49214959

4922-
if (_Pickler_SetProtocol(self, protocol, fix_imports) < 0)
4923-
return -1;
4924-
4925-
if (_Pickler_SetOutputStream(self, file) < 0)
4926-
return -1;
4927-
4928-
if (_Pickler_SetBufferCallback(self, buffer_callback) < 0)
4929-
return -1;
4930-
4960+
if (_Pickler_SetProtocol(self, protocol, fix_imports) < 0) {
4961+
goto error;
4962+
}
4963+
if (_Pickler_SetOutputStream(self, file) < 0) {
4964+
goto error;
4965+
}
4966+
if (_Pickler_SetBufferCallback(self, buffer_callback) < 0) {
4967+
goto error;
4968+
}
49314969
/* memo and output_buffer may have already been created in _Pickler_New */
49324970
if (self->memo == NULL) {
49334971
self->memo = PyMemoTable_New();
4934-
if (self->memo == NULL)
4935-
return -1;
4972+
if (self->memo == NULL) {
4973+
goto error;
4974+
}
49364975
}
49374976
self->output_len = 0;
49384977
if (self->output_buffer == NULL) {
49394978
self->max_output_len = WRITE_BUF_SIZE;
49404979
self->output_buffer = PyBytes_FromStringAndSize(NULL,
49414980
self->max_output_len);
4942-
if (self->output_buffer == NULL)
4943-
return -1;
4981+
if (self->output_buffer == NULL) {
4982+
goto error;
4983+
}
49444984
}
49454985

49464986
self->fast = 0;
49474987
self->fast_nesting = 0;
49484988
self->fast_memo = NULL;
49494989

4950-
if (self->dispatch_table != NULL) {
4951-
return 0;
4952-
}
4953-
if (PyObject_GetOptionalAttr((PyObject *)self, &_Py_ID(dispatch_table),
4954-
&self->dispatch_table) < 0) {
4955-
return -1;
4990+
if (self->dispatch_table == NULL) {
4991+
if (PyObject_GetOptionalAttr((PyObject *)self, &_Py_ID(dispatch_table),
4992+
&self->dispatch_table) < 0) {
4993+
goto error;
4994+
}
49564995
}
49574996

4997+
END_USING_PICKLER(self);
49584998
return 0;
4999+
5000+
error:
5001+
END_USING_PICKLER(self);
5002+
return -1;
49595003
}
49605004

49615005

@@ -7157,22 +7201,22 @@ static PyObject *
71577201
_pickle_Unpickler_load_impl(UnpicklerObject *self, PyTypeObject *cls)
71587202
/*[clinic end generated code: output=cc88168f608e3007 input=f5d2f87e61d5f07f]*/
71597203
{
7160-
UnpicklerObject *unpickler = (UnpicklerObject*)self;
7161-
71627204
PickleState *st = _Pickle_GetStateByClass(cls);
71637205

71647206
/* Check whether the Unpickler was initialized correctly. This prevents
71657207
segfaulting if a subclass overridden __init__ with a function that does
71667208
not call Unpickler.__init__(). Here, we simply ensure that self->read
71677209
is not NULL. */
7168-
if (unpickler->read == NULL) {
7210+
if (self->read == NULL) {
71697211
PyErr_Format(st->UnpicklingError,
71707212
"Unpickler.__init__() was not called by %s.__init__()",
7171-
Py_TYPE(unpickler)->tp_name);
7213+
Py_TYPE(self)->tp_name);
71727214
return NULL;
71737215
}
7174-
7175-
return load(st, unpickler);
7216+
BEGIN_USING_UNPICKLER(self, NULL);
7217+
PyObject *res = load(st, self);
7218+
END_USING_UNPICKLER(self);
7219+
return res;
71767220
}
71777221

71787222
/* The name of find_class() is misleading. In newer pickle protocols, this
@@ -7436,35 +7480,41 @@ _pickle_Unpickler___init___impl(UnpicklerObject *self, PyObject *file,
74367480
const char *errors, PyObject *buffers)
74377481
/*[clinic end generated code: output=09f0192649ea3f85 input=ca4c1faea9553121]*/
74387482
{
7483+
BEGIN_USING_UNPICKLER(self, -1);
74397484
/* In case of multiple __init__() calls, clear previous content. */
74407485
if (self->read != NULL)
74417486
(void)Unpickler_clear((PyObject *)self);
74427487

7443-
if (_Unpickler_SetInputStream(self, file) < 0)
7444-
return -1;
7445-
7446-
if (_Unpickler_SetInputEncoding(self, encoding, errors) < 0)
7447-
return -1;
7448-
7449-
if (_Unpickler_SetBuffers(self, buffers) < 0)
7450-
return -1;
7451-
7488+
if (_Unpickler_SetInputStream(self, file) < 0) {
7489+
goto error;
7490+
}
7491+
if (_Unpickler_SetInputEncoding(self, encoding, errors) < 0) {
7492+
goto error;
7493+
}
7494+
if (_Unpickler_SetBuffers(self, buffers) < 0) {
7495+
goto error;
7496+
}
74527497
self->fix_imports = fix_imports;
74537498

74547499
PyTypeObject *tp = Py_TYPE(self);
74557500
PickleState *state = _Pickle_FindStateByType(tp);
74567501
self->stack = (Pdata *)Pdata_New(state);
7457-
if (self->stack == NULL)
7458-
return -1;
7459-
7502+
if (self->stack == NULL) {
7503+
goto error;
7504+
}
74607505
self->memo_size = 32;
74617506
self->memo = _Unpickler_NewMemo(self->memo_size);
7462-
if (self->memo == NULL)
7463-
return -1;
7464-
7507+
if (self->memo == NULL) {
7508+
goto error;
7509+
}
74657510
self->proto = 0;
74667511

7512+
END_USING_UNPICKLER(self);
74677513
return 0;
7514+
7515+
error:
7516+
END_USING_UNPICKLER(self);
7517+
return -1;
74687518
}
74697519

74707520

0 commit comments

Comments
 (0)