Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Lib/test/pickletester.py
Original file line number Diff line number Diff line change
Expand Up @@ -4261,6 +4261,34 @@ def check_array(arr):
# 2-D, non-contiguous
check_array(arr[::2])

def test_concurrent_mutation_in_buffer_with_bytearray(self):
def factory():
s = b"a" * 16
return bytearray(s), s
self.do_test_concurrent_mutation_in_buffer_callback(factory)

def test_concurrent_mutation_in_buffer_with_memoryview(self):
def factory():
obj = memoryview(b"a" * 32)[10:26]
sub = b"a" * len(obj)
return obj, sub
self.do_test_concurrent_mutation_in_buffer_callback(factory)

def do_test_concurrent_mutation_in_buffer_callback(self, factory):
class R:
def __bool__(self):
buf.release()
return True

for proto in range(5, pickle.HIGHEST_PROTOCOL + 1):
obj, sub = factory()
buf = pickle.PickleBuffer(obj)
buffer_callback = lambda _: R()

with self.subTest(proto=proto, obj=obj, sub=sub):
res = self.dumps(buf, proto, buffer_callback=buffer_callback)
self.assertIn(sub, res)

def test_evil_class_mutating_dict(self):
# https://github.com/python/cpython/issues/92930
from random import getrandbits
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:mod:`pickle`: fix use-after-free crashes when a :class:`~pickle.PickleBuffer`
is concurrently mutated by a custom buffer callback during pickling.
Patch by Bénédikt Tran and Aaron Wieczorek.
29 changes: 22 additions & 7 deletions Modules/_pickle.c
Original file line number Diff line number Diff line change
Expand Up @@ -2646,43 +2646,58 @@ save_picklebuffer(PickleState *st, PicklerObject *self, PyObject *obj)
"pointing to a non-contiguous buffer");
return -1;
}

int rc = 0;
Py_buffer keepalive_view; // to ensure that 'view' is kept alive
if (PyObject_GetBuffer(view->obj, &keepalive_view, PyBUF_FULL_RO) != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could not we simply apply PyObject_GetBuffer() to the PickleBuffer object itself? We can then get rid of PyPickleBuffer_GetBuffer().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially! I'd want to test it. @picnixz what do you think? Since this hasn't been merged yet now would seem to be the time.

Copy link
Member

@picnixz picnixz Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should work. PyPickleBuffer_GetBuffer just returns a (borrowed) ref so it doesn't get any buffer on it. I think it should work so feel free to check!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked great, thanks for the suggestion @serhiy-storchaka. @picnixz The change was a little more involved than expected, but by doing this I was able to remove a redundant view, update all the view-> calls, and the exit: label. All tests pass and confirmed that the patch still holds.

return -1;
}

int in_band = 1;
if (self->buffer_callback != NULL) {
PyObject *ret = PyObject_CallOneArg(self->buffer_callback, obj);
if (ret == NULL) {
return -1;
goto error;
}
in_band = PyObject_IsTrue(ret);
Py_DECREF(ret);
if (in_band == -1) {
return -1;
goto error;
}
}
if (in_band) {
/* Write data in-band */
if (view->readonly) {
return _save_bytes_data(st, self, obj, (const char *)view->buf,
rc = _save_bytes_data(st, self, obj, (const char *)view->buf,
view->len);
}
else {
return _save_bytearray_data(st, self, obj, (const char *)view->buf,
rc = _save_bytearray_data(st, self, obj, (const char *)view->buf,
view->len);
}
goto exit;
}
else {
/* Write data out-of-band */
const char next_buffer_op = NEXT_BUFFER;
if (_Pickler_Write(self, &next_buffer_op, 1) < 0) {
return -1;
goto error;
}
if (view->readonly) {
const char readonly_buffer_op = READONLY_BUFFER;
if (_Pickler_Write(self, &readonly_buffer_op, 1) < 0) {
return -1;
goto error;
}
}
}
return 0;

exit:
PyBuffer_Release(&keepalive_view);
return rc;

error:
PyBuffer_Release(&keepalive_view);
return -1;
}

/* A copy of PyUnicode_AsRawUnicodeEscapeString() that also translates
Expand Down
Loading