Skip to content

Commit d5c1b29

Browse files
stestaggcmaloney
andauthored
gh-153419: Fix several issues around bytearray __init__ (#153498)
Introduce a bytearray_new() function to ensure that ob_bytes_object is always set on a bytearray. Resizing a bytearray to 0 length now explicitly sets the ob_bytes_object to the empty constant immortal. Add a check in the 'bytearray init from string' fast path to ensure there are no active exports. This fixes asserts/crashes on the following: - bytearray(1).__init__() - bytearray().__new__(bytearray).append(1) - a = bytearray(); b = memoryview(a); a.__init__('x', 'ascii') Co-authored-by: Cody Maloney <cmaloney@users.noreply.github.com>
1 parent d24d9d0 commit d5c1b29

4 files changed

Lines changed: 78 additions & 12 deletions

File tree

Lib/test/test_bytes.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2226,6 +2226,46 @@ def __len__(self):
22262226

22272227
self.assertRaises(BufferError, ba.hex, S(b':'))
22282228

2229+
def test_no_init_called(self):
2230+
# A bytearray created without calling bytearray.__init__
2231+
# should not crash the interpreter (see gh-153419).
2232+
def bytearray_new():
2233+
return bytearray.__new__(bytearray)
2234+
2235+
bytearray_new().insert(0, 1)
2236+
bytearray_new().extend(b"x")
2237+
bytearray_new().extend([1, 2, 3])
2238+
bytearray_new().resize(4)
2239+
bytearray_new().__init__(5)
2240+
bytearray_new().__init__(b"xyz")
2241+
bytearray_new().take_bytes()
2242+
bytearray_new().take_bytes(0)
2243+
2244+
a = bytearray_new()
2245+
a.append(1)
2246+
2247+
a = bytearray_new()
2248+
a += b"x"
2249+
2250+
a = bytearray_new()
2251+
a[:] = b"xyz"
2252+
2253+
def test_reinit_length(self):
2254+
# There is a shortcut taken when resizing, where alloc/2 < newsize.
2255+
# In this case, the existing buffer is reused, rather than reset.
2256+
# If this happens when newsize == 0 and alloc == 1, then various
2257+
# code assumptions can be violated. This test should catch those
2258+
# in debug builds. (see gh-153419)
2259+
a = bytearray(1)
2260+
a.__init__()
2261+
self.assertEqual(a, b"")
2262+
2263+
def test_reinit_with_view(self):
2264+
a = bytearray()
2265+
with memoryview(a):
2266+
self.assertRaises(BufferError, a.__init__, "x", "ascii")
2267+
self.assertEqual(a, b"")
2268+
22292269

22302270
class AssortedBytesTest(unittest.TestCase):
22312271
#
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix multiple :class:`bytearray` crashes and reference leaks caused by skipping :meth:`~object.__init__` and broken state setup code.

Objects/bytearrayobject.c

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,9 @@ bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
213213
{
214214
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self);
215215
PyByteArrayObject *obj = ((PyByteArrayObject *)self);
216+
217+
assert(obj->ob_bytes_object != NULL);
218+
216219
/* All computations are done unsigned to avoid integer overflows
217220
(see issue #22335). */
218221
size_t alloc = (size_t) obj->ob_alloc;
@@ -236,6 +239,14 @@ bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
236239
return -1;
237240
}
238241

242+
/* Resize to 0 resets to empty bytes (see issue #153419). */
243+
if (requested_size == 0) {
244+
Py_SETREF(obj->ob_bytes_object,
245+
Py_GetConstant(Py_CONSTANT_EMPTY_BYTES));
246+
bytearray_reinit_from_bytes(obj, 0, 0);
247+
return 0;
248+
}
249+
239250
if (size + logical_offset <= alloc) {
240251
/* Current buffer is large enough to host the requested size,
241252
decide on a strategy. */
@@ -902,6 +913,20 @@ bytearray_ass_subscript(PyObject *op, PyObject *index, PyObject *values)
902913
return ret;
903914
}
904915

916+
static PyObject *
917+
bytearray_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
918+
{
919+
PyObject *op = PyType_GenericNew(type, args, kwds);
920+
if (op == NULL) {
921+
return NULL;
922+
}
923+
PyByteArrayObject *self = _PyByteArray_CAST(op);
924+
self->ob_bytes_object = Py_GetConstant(Py_CONSTANT_EMPTY_BYTES);
925+
bytearray_reinit_from_bytes(self, 0, 0);
926+
self->ob_exports = 0;
927+
return op;
928+
}
929+
905930
/*[clinic input]
906931
bytearray.__init__
907932
@@ -920,20 +945,16 @@ bytearray___init___impl(PyByteArrayObject *self, PyObject *arg,
920945
PyObject *it;
921946
PyObject *(*iternext)(PyObject *);
922947

923-
/* First __init__; set ob_bytes_object so ob_bytes is always non-null. */
924-
if (self->ob_bytes_object == NULL) {
925-
self->ob_bytes_object = Py_GetConstant(Py_CONSTANT_EMPTY_BYTES);
926-
bytearray_reinit_from_bytes(self, 0, 0);
927-
self->ob_exports = 0;
948+
/* Disallow any __init__ call if the object is not resizable (has exports)
949+
to make the handling of non-null `source` init values simpler. */
950+
if (!_canresize(self)) {
951+
return -1;
928952
}
929953

930-
if (Py_SIZE(self) != 0) {
931-
/* Empty previous contents (yes, do this first of all!) */
932-
if (PyByteArray_Resize((PyObject *)self, 0) < 0)
933-
return -1;
954+
/* Empty any previous contents (do this first of all!). */
955+
if (PyByteArray_Resize((PyObject *)self, 0) < 0) {
956+
return -1;
934957
}
935-
936-
/* Should be caused by first init or the resize to 0. */
937958
assert(self->ob_bytes_object == Py_GetConstantBorrowed(Py_CONSTANT_EMPTY_BYTES));
938959
assert(self->ob_exports == 0);
939960

@@ -1609,6 +1630,9 @@ bytearray_take_bytes_impl(PyByteArrayObject *self, PyObject *n)
16091630
}
16101631

16111632
if (_PyBytes_Resize(&self->ob_bytes_object, to_take) == -1) {
1633+
assert(self->ob_bytes_object == NULL);
1634+
self->ob_bytes_object = Py_GetConstant(Py_CONSTANT_EMPTY_BYTES);
1635+
bytearray_reinit_from_bytes(self, 0, 0);
16121636
Py_DECREF(remaining);
16131637
return NULL;
16141638
}
@@ -2939,7 +2963,7 @@ PyTypeObject PyByteArray_Type = {
29392963
0, /* tp_dictoffset */
29402964
bytearray___init__, /* tp_init */
29412965
PyType_GenericAlloc, /* tp_alloc */
2942-
PyType_GenericNew, /* tp_new */
2966+
bytearray_new, /* tp_new */
29432967
PyObject_Free, /* tp_free */
29442968
.tp_version_tag = _Py_TYPE_VERSION_BYTEARRAY,
29452969
};

Objects/bytesobject.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3380,6 +3380,7 @@ _PyBytes_Resize(PyObject **pv, Py_ssize_t newsize)
33803380
Py_DECREF(v);
33813381
return (*pv == NULL) ? -1 : 0;
33823382
}
3383+
assert(v != bytes_get_empty());
33833384

33843385
#ifdef Py_TRACE_REFS
33853386
_Py_ForgetReference(v);

0 commit comments

Comments
 (0)