Skip to content

Commit c1a4112

Browse files
gh-147988: Initialize digits in long_alloc() in debug mode (#147989)
When Python is built in debug mode: * long_alloc() now initializes digits with a pattern to detect usage of uninitialized digits. * _PyLong_CompactValue() now makes sure that the digit is zero when the sign is zero. * PyLongWriter_Finish() now raises SystemError if it detects uninitialized digits Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent a86963b commit c1a4112

File tree

4 files changed

+71
-7
lines changed

4 files changed

+71
-7
lines changed

Include/cpython/longintrepr.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,11 @@ _PyLong_CompactValue(const PyLongObject *op)
133133
assert(PyType_HasFeature(op->ob_base.ob_type, Py_TPFLAGS_LONG_SUBCLASS));
134134
assert(PyUnstable_Long_IsCompact(op));
135135
sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK);
136+
if (sign == 0) {
137+
// gh-147988: Make sure that the digit is zero.
138+
// It helps detecting the usage of uninitialized digits.
139+
assert(op->long_value.ob_digit[0] == 0);
140+
}
136141
return sign * (Py_ssize_t)op->long_value.ob_digit[0];
137142
}
138143

Lib/test/test_capi/test_long.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,16 @@ def to_digits(num):
803803
self.assertEqual(pylongwriter_create(negative, digits), num,
804804
(negative, digits))
805805

806+
@unittest.skipUnless(support.Py_DEBUG, "need a debug build (Py_DEBUG)")
807+
def test_longwriter_finish(self):
808+
# Test PyLongWriter_Create(0, 3, &digits) with PyLongWriter_Finish()
809+
# where the last digit is left uninitialized
810+
pylongwriter_finish_bug = _testcapi.pylongwriter_finish_bug
811+
with self.assertRaises(SystemError) as cm:
812+
pylongwriter_finish_bug()
813+
self.assertEqual(str(cm.exception),
814+
'PyLongWriter_Finish: digit 2 is uninitialized')
815+
806816
def test_bug_143050(self):
807817
with support.adjust_int_max_str_digits(0):
808818
# Bug coming from using _pylong.int_from_string(), that

Modules/_testcapi/long.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,25 @@ pylongwriter_create(PyObject *module, PyObject *args)
254254
}
255255

256256

257+
static PyObject *
258+
pylongwriter_finish_bug(PyObject *module, PyObject *Py_UNUSED(args))
259+
{
260+
void *writer_digits;
261+
PyLongWriter *writer = PyLongWriter_Create(0, 3, &writer_digits);
262+
if (writer == NULL) {
263+
return NULL;
264+
}
265+
266+
assert(PyLong_GetNativeLayout()->digit_size == sizeof(digit));
267+
digit *digits = writer_digits;
268+
digits[0] = 1;
269+
digits[1] = 1;
270+
// Oops, digits[2] is left uninitialized on purpose
271+
// to test PyLongWriter_Finish()
272+
return PyLongWriter_Finish(writer);
273+
}
274+
275+
257276
static PyObject *
258277
get_pylong_layout(PyObject *module, PyObject *Py_UNUSED(args))
259278
{
@@ -271,6 +290,7 @@ static PyMethodDef test_methods[] = {
271290
{"pylong_aspid", pylong_aspid, METH_O},
272291
{"pylong_export", pylong_export, METH_O},
273292
{"pylongwriter_create", pylongwriter_create, METH_VARARGS},
293+
{"pylongwriter_finish_bug", pylongwriter_finish_bug, METH_NOARGS},
274294
{"get_pylong_layout", get_pylong_layout, METH_NOARGS},
275295
{"pylong_ispositive", pylong_ispositive, METH_O},
276296
{"pylong_isnegative", pylong_isnegative, METH_O},

Objects/longobject.c

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,11 @@ long_alloc(Py_ssize_t size)
188188
_PyLong_InitTag(result);
189189
}
190190
_PyLong_SetSignAndDigitCount(result, size != 0, size);
191-
/* The digit has to be initialized explicitly to avoid
192-
* use-of-uninitialized-value. */
193-
result->long_value.ob_digit[0] = 0;
191+
#ifdef Py_DEBUG
192+
// gh-147988: Fill digits with an invalid pattern to catch usage
193+
// of uninitialized digits.
194+
memset(result->long_value.ob_digit, 0xFF, ndigits * sizeof(digit));
195+
#endif
194196
return result;
195197
}
196198

@@ -1097,6 +1099,7 @@ _PyLong_FromByteArray(const unsigned char* bytes, size_t n,
10971099
int sign = is_signed ? -1: 1;
10981100
if (idigit == 0) {
10991101
sign = 0;
1102+
v->long_value.ob_digit[0] = 0;
11001103
}
11011104
_PyLong_SetSignAndDigitCount(v, sign, idigit);
11021105
return (PyObject *)maybe_small_long(long_normalize(v));
@@ -2855,6 +2858,7 @@ long_from_non_binary_base(const char *start, const char *end, Py_ssize_t digits,
28552858
*res = NULL;
28562859
return 0;
28572860
}
2861+
z->long_value.ob_digit[0] = 0;
28582862
_PyLong_SetSignAndDigitCount(z, 0, 0);
28592863

28602864
/* `convwidth` consecutive input digits are treated as a single
@@ -3368,6 +3372,7 @@ x_divrem(PyLongObject *v1, PyLongObject *w1, PyLongObject **prem)
33683372
*prem = NULL;
33693373
return NULL;
33703374
}
3375+
a->long_value.ob_digit[0] = 0;
33713376
v0 = v->long_value.ob_digit;
33723377
w0 = w->long_value.ob_digit;
33733378
wm1 = w0[size_w-1];
@@ -4144,10 +4149,6 @@ k_mul(PyLongObject *a, PyLongObject *b)
41444149
/* 1. Allocate result space. */
41454150
ret = long_alloc(asize + bsize);
41464151
if (ret == NULL) goto fail;
4147-
#ifdef Py_DEBUG
4148-
/* Fill with trash, to catch reference to uninitialized digits. */
4149-
memset(ret->long_value.ob_digit, 0xDF, _PyLong_DigitCount(ret) * sizeof(digit));
4150-
#endif
41514152

41524153
/* 2. t1 <- ah*bh, and copy into high digits of result. */
41534154
if ((t1 = k_mul(ah, bh)) == NULL) goto fail;
@@ -5636,6 +5637,12 @@ long_bitwise(PyLongObject *a,
56365637
Py_UNREACHABLE();
56375638
}
56385639

5640+
if ((size_z + negz) == 0) {
5641+
Py_XDECREF(new_a);
5642+
Py_XDECREF(new_b);
5643+
return get_small_int(0);
5644+
}
5645+
56395646
/* We allow an extra digit if z is negative, to make sure that
56405647
the final two's complement of z doesn't overflow. */
56415648
z = long_alloc(size_z + negz);
@@ -6959,6 +6966,28 @@ PyLongWriter_Finish(PyLongWriter *writer)
69596966
PyLongObject *obj = (PyLongObject *)writer;
69606967
assert(Py_REFCNT(obj) == 1);
69616968

6969+
#ifdef Py_DEBUG
6970+
// gh-147988: Detect uninitialized digits: long_alloc() fills digits with
6971+
// 0xFF byte pattern. It's posssible because PyLong_BASE is smaller than
6972+
// the maximum value of the C digit type (uint32_t or unsigned short):
6973+
// most significan bits are unused by the API.
6974+
Py_ssize_t ndigits = _PyLong_DigitCount(obj);
6975+
if (ndigits == 0) {
6976+
// Check ob_digit[0] digit for the number zero
6977+
ndigits = 1;
6978+
}
6979+
for (Py_ssize_t i = 0; i < ndigits; i++) {
6980+
digit d = obj->long_value.ob_digit[i];
6981+
if (d & ~(digit)PyLong_MASK) {
6982+
Py_DECREF(obj);
6983+
PyErr_Format(PyExc_SystemError,
6984+
"PyLongWriter_Finish: digit %zd is uninitialized",
6985+
i);
6986+
return NULL;
6987+
}
6988+
}
6989+
#endif
6990+
69626991
// Normalize and get singleton if possible
69636992
obj = maybe_small_long(long_normalize(obj));
69646993

0 commit comments

Comments
 (0)