Skip to content

Commit 9a52edb

Browse files
[3.15] gh-160303: Fix bytearray state after a failed del b[:n] (GH-160304) (#161836)
After a failed `del b[:n]`, `bytearray_setslice_linear()` moved `ob_start` back, but `bytearray_resize_storage()` had already realigned the data and reset `ob_start`, so the bytearray then started `n` bytes before its buffer. The front-delete path now moves `ob_start` and the size before the shrinking resize and no longer restores anything, so a failed shrink leaves the bytes removed and the larger block in place, raising `MemoryError` as the non-front path already does. The checks in `bytearray_resize_lock_held()` are split from a new `bytearray_resize_unchecked()` so the slice code can call it directly without changing other callers, and the non-front failure path now writes the trailing null byte it was missing. Fixes #160303 ANT-2026-QRZ6IDTC <!-- gh-issue-number: gh-160303 --> * Issue: gh-160303 <!-- /gh-issue-number --> (cherry picked from commit 35e2c5a) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent f2a3355 commit 9a52edb

3 files changed

Lines changed: 91 additions & 38 deletions

File tree

‎Lib/test/test_bytes.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1590,6 +1590,10 @@ def test_resize(self):
15901590
def test_resize_error(self):
15911591
# gh-157242: If bytearray.resize() fails (MemoryError),
15921592
# the bytearray must be left unchanged.
1593+
#
1594+
# gh-160303: If a slice is deleted or replaced by shorter data and
1595+
# shrinking the memory block fails (MemoryError), the bytearray
1596+
# must be modified anyway.
15931597

15941598
offset = 3
15951599
for logical_offset in (False, True):
@@ -1616,6 +1620,33 @@ def test_resize_error(self):
16161620
ba.resize(1)
16171621
self.assertEqual(ba, expected)
16181622

1623+
# shrink bytearray with a slice at the front, in the middle
1624+
# and at the end
1625+
data = b'0123456789'
1626+
if logical_offset:
1627+
data = data[offset:]
1628+
for start, stop in ((0, -2), (1, -1), (2, len(data))):
1629+
for replacement in (b'', b'.'):
1630+
with self.subTest(start=start, stop=stop,
1631+
replacement=replacement):
1632+
ba = bytearray(b'0123456789')
1633+
if logical_offset:
1634+
del ba[:offset]
1635+
index = slice(start, stop)
1636+
value = bytearray(replacement)
1637+
expected = data[:start] + replacement + data[stop:]
1638+
with inject_memory_error(self):
1639+
if replacement:
1640+
ba[index] = value
1641+
else:
1642+
del ba[index]
1643+
self.assertEqual(ba, expected)
1644+
# float() relies on the null byte after the data.
1645+
self.assertEqual(float(ba), float(expected))
1646+
ba.append(ord('5'))
1647+
del ba[:1]
1648+
self.assertEqual(ba, expected[1:] + b'5')
1649+
16191650
def test_take_bytes(self):
16201651
ba = bytearray(b'ab')
16211652
self.assertEqual(ba.take_bytes(), b'ab')
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix :class:`bytearray` reading and writing memory outside of its buffer
2+
after ``del b[:n]`` or ``b[:n] = shorter`` failed with :exc:`MemoryError`.
3+
The bytes are now removed even if shrinking the memory block fails;
4+
:exc:`MemoryError` is still raised. Also fix the missing null byte after the
5+
data when shrinking fails for a slice which is not at the front.

‎Objects/bytearrayobject.c‎

Lines changed: 55 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -260,37 +260,31 @@ bytearray_resize_storage(PyByteArrayObject *self,
260260
}
261261

262262

263+
/* Resize without the checks of bytearray_resize_lock_held(): the caller must
264+
have checked that the bytearray can be resized.
265+
266+
Unlike bytearray_resize_lock_held(), this does not return early if
267+
requested_size is already the size of the bytearray. A caller which
268+
removed bytes by advancing ob_start and reducing the size uses that to
269+
release the memory. */
263270
static int
264-
bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
271+
bytearray_resize_unchecked(PyByteArrayObject *obj, Py_ssize_t requested_size)
265272
{
266-
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self);
267-
PyByteArrayObject *obj = ((PyByteArrayObject *)self);
273+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj);
274+
PyObject *self = (PyObject *)obj;
268275

269276
assert(obj->ob_bytes_object != NULL);
277+
assert(requested_size >= 0);
278+
assert(obj->ob_exports == 0);
270279

271280
/* All computations are done unsigned to avoid integer overflows
272281
(see issue #22335). */
273282
size_t alloc = (size_t) obj->ob_alloc;
274283
size_t logical_offset = (size_t) (obj->ob_start - obj->ob_bytes);
275284
size_t size = (size_t) requested_size;
276285

277-
assert(self != NULL);
278-
assert(PyByteArray_Check(self));
279286
assert(logical_offset <= alloc);
280287

281-
if (requested_size < 0) {
282-
PyErr_Format(PyExc_ValueError,
283-
"Can only resize to positive sizes, got %zd", requested_size);
284-
return -1;
285-
}
286-
287-
if (requested_size == Py_SIZE(self)) {
288-
return 0;
289-
}
290-
if (!_canresize(obj)) {
291-
return -1;
292-
}
293-
294288
/* Resize to 0 resets to empty bytes (see issue #153419). */
295289
if (requested_size == 0) {
296290
Py_SETREF(obj->ob_bytes_object,
@@ -342,6 +336,30 @@ bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
342336
return 0;
343337
}
344338

339+
static int
340+
bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
341+
{
342+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self);
343+
PyByteArrayObject *obj = ((PyByteArrayObject *)self);
344+
345+
assert(self != NULL);
346+
assert(PyByteArray_Check(self));
347+
348+
if (requested_size < 0) {
349+
PyErr_Format(PyExc_ValueError,
350+
"Can only resize to positive sizes, got %zd", requested_size);
351+
return -1;
352+
}
353+
354+
if (requested_size == Py_SIZE(self)) {
355+
return 0;
356+
}
357+
if (!_canresize(obj)) {
358+
return -1;
359+
}
360+
return bytearray_resize_unchecked(obj, requested_size);
361+
}
362+
345363
int
346364
PyByteArray_Resize(PyObject *self, Py_ssize_t requested_size)
347365
{
@@ -615,14 +633,22 @@ bytearray_setslice_linear(PyByteArrayObject *self,
615633
return -1;
616634

617635
if (lo == 0) {
618-
/* Shrink the buffer by advancing its logical start */
636+
/* Shrink the buffer by advancing its logical start. The bytes
637+
are removed at this point; the resize below only releases
638+
memory. */
619639
self->ob_start -= growth;
640+
Py_SET_SIZE(self, Py_SIZE(self) + growth);
620641
/*
621642
0 lo hi old_size
622643
| |<----avail----->|<-----tail------>|
623644
| |<-bytes_len->|<-----tail------>|
624645
0 new_lo new_hi new_size
625646
*/
647+
if (bytearray_resize_unchecked(self, Py_SIZE(self)) < 0) {
648+
/* The operation is completed, but a MemoryError is still
649+
raised and the memory block is not shrunk. */
650+
res = -1;
651+
}
626652
}
627653
else {
628654
/*
@@ -633,26 +659,17 @@ bytearray_setslice_linear(PyByteArrayObject *self,
633659
*/
634660
memmove(buf + lo + bytes_len, buf + hi,
635661
Py_SIZE(self) - hi);
636-
}
637-
if (bytearray_resize_lock_held((PyObject *)self,
638-
Py_SIZE(self) + growth) < 0) {
639-
/* Issue #19578: Handling the memory allocation failure here is
640-
tricky here because the bytearray object has already been
641-
modified. Depending on growth and lo, the behaviour is
642-
different.
643-
644-
If growth < 0 and lo != 0, the operation is completed, but a
645-
MemoryError is still raised and the memory block is not
646-
shrunk. Otherwise, the bytearray is restored in its previous
647-
state and a MemoryError is raised. */
648-
if (lo == 0) {
649-
self->ob_start += growth;
650-
return -1;
662+
if (bytearray_resize_lock_held((PyObject *)self,
663+
Py_SIZE(self) + growth) < 0) {
664+
/* Issue #19578: memmove() removed bytes, the bytearray
665+
object cannot be restored in its previous state.
666+
667+
The operation is completed, but a MemoryError is still
668+
raised and the memory block is not shrunk. */
669+
Py_SET_SIZE(self, Py_SIZE(self) + growth);
670+
bytearray_write_trailing_null_byte(self);
671+
res = -1;
651672
}
652-
/* memmove() removed bytes, the bytearray object cannot be
653-
restored in its previous state. */
654-
Py_SET_SIZE(self, Py_SIZE(self) + growth);
655-
res = -1;
656673
}
657674
buf = PyByteArray_AS_STRING(self);
658675
}

0 commit comments

Comments
 (0)