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>

diff --git a/Include/cpython/longintrepr.h b/Include/cpython/longintrepr.h
index 804c1e9427e063f91f2fc1b6e621eed9e7a8cdc2..998ebe6891577e80aa176d7263dc0927c2740716 100644 (file)
--- a/Include/cpython/longintrepr.h
+++ b/Include/cpython/longintrepr.h
@@ -133,6 +133,11 @@ _PyLong_CompactValue(const PyLongObject *op)
     assert(PyType_HasFeature(op->ob_base.ob_type, Py_TPFLAGS_LONG_SUBCLASS));
     assert(PyUnstable_Long_IsCompact(op));
     sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK);
+    if (sign == 0) {
+        // gh-147988: Make sure that the digit is zero.
+        // It helps detecting the usage of uninitialized digits.
+        assert(op->long_value.ob_digit[0] == 0);
+    }
     return sign * (Py_ssize_t)op->long_value.ob_digit[0];
 }
 

diff --git a/Lib/test/test_capi/test_long.py b/Lib/test/test_capi/test_long.py
index fc0454b71cb780de68ae38cc88518abe873153e2..d1467caf6880a614ca070e3af4062effcd41f856 100644 (file)
--- a/Lib/test/test_capi/test_long.py
+++ b/Lib/test/test_capi/test_long.py
@@ -803,6 +803,16 @@ class LongTests(unittest.TestCase):
                 self.assertEqual(pylongwriter_create(negative, digits), num,
                                  (negative, digits))
 
+    @unittest.skipUnless(support.Py_DEBUG, "need a debug build (Py_DEBUG)")
+    def test_longwriter_finish(self):
+        # Test PyLongWriter_Create(0, 3, &digits) with PyLongWriter_Finish()
+        # where the last digit is left uninitialized
+        pylongwriter_finish_bug = _testcapi.pylongwriter_finish_bug
+        with self.assertRaises(SystemError) as cm:
+            pylongwriter_finish_bug()
+        self.assertEqual(str(cm.exception),
+                         'PyLongWriter_Finish: digit 2 is uninitialized')
+
     def test_bug_143050(self):
         with support.adjust_int_max_str_digits(0):
             # Bug coming from using _pylong.int_from_string(), that

diff --git a/Modules/_testcapi/long.c b/Modules/_testcapi/long.c
index 6313abf5485fff1e3c3788c44bb5edd7707e961e..008a7d37726869b094742be9bc930fb1b6bb97be 100644 (file)
--- a/Modules/_testcapi/long.c
+++ b/Modules/_testcapi/long.c
@@ -254,6 +254,25 @@ error:
 }
 
 
+static PyObject *
+pylongwriter_finish_bug(PyObject *module, PyObject *Py_UNUSED(args))
+{
+    void *writer_digits;
+    PyLongWriter *writer = PyLongWriter_Create(0, 3, &writer_digits);
+    if (writer == NULL) {
+        return NULL;
+    }
+
+    assert(PyLong_GetNativeLayout()->digit_size == sizeof(digit));
+    digit *digits = writer_digits;
+    digits[0] = 1;
+    digits[1] = 1;
+    // Oops, digits[2] is left uninitialized on purpose
+    // to test PyLongWriter_Finish()
+    return PyLongWriter_Finish(writer);
+}
+
+
 static PyObject *
 get_pylong_layout(PyObject *module, PyObject *Py_UNUSED(args))
 {
@@ -271,6 +290,7 @@ static PyMethodDef test_methods[] = {
     {"pylong_aspid",                pylong_aspid,               METH_O},
     {"pylong_export",               pylong_export,              METH_O},
     {"pylongwriter_create",         pylongwriter_create,        METH_VARARGS},
+    {"pylongwriter_finish_bug",     pylongwriter_finish_bug,    METH_NOARGS},
     {"get_pylong_layout",           get_pylong_layout,          METH_NOARGS},
     {"pylong_ispositive",           pylong_ispositive,          METH_O},
     {"pylong_isnegative",           pylong_isnegative,          METH_O},

diff --git a/Objects/longobject.c b/Objects/longobject.c
index fde01ff302d3c2bc9b6157c90a1fbd5320f00a0e..549cf0b8f12b4e482024d0524fba6483d04268b1 100644 (file)
--- a/Objects/longobject.c
+++ b/Objects/longobject.c
@@ -188,9 +188,11 @@ long_alloc(Py_ssize_t size)
         _PyLong_InitTag(result);
     }
     _PyLong_SetSignAndDigitCount(result, size != 0, size);
-    /* The digit has to be initialized explicitly to avoid
-     * use-of-uninitialized-value. */
-    result->long_value.ob_digit[0] = 0;
+#ifdef Py_DEBUG
+    // gh-147988: Fill digits with an invalid pattern to catch usage
+    // of uninitialized digits.
+    memset(result->long_value.ob_digit, 0xFF, ndigits * sizeof(digit));
+#endif
     return result;
 }
 
@@ -1097,6 +1099,7 @@ _PyLong_FromByteArray(const unsigned char* bytes, size_t n,
     int sign = is_signed ? -1: 1;
     if (idigit == 0) {
         sign = 0;
+        v->long_value.ob_digit[0] = 0;
     }
     _PyLong_SetSignAndDigitCount(v, sign, idigit);
     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,
         *res = NULL;
         return 0;
     }
+    z->long_value.ob_digit[0] = 0;
     _PyLong_SetSignAndDigitCount(z, 0, 0);
 
     /* `convwidth` consecutive input digits are treated as a single
@@ -3368,6 +3372,7 @@ x_divrem(PyLongObject *v1, PyLongObject *w1, PyLongObject **prem)
         *prem = NULL;
         return NULL;
     }
+    a->long_value.ob_digit[0] = 0;
     v0 = v->long_value.ob_digit;
     w0 = w->long_value.ob_digit;
     wm1 = w0[size_w-1];
@@ -4144,10 +4149,6 @@ k_mul(PyLongObject *a, PyLongObject *b)
     /* 1. Allocate result space. */
     ret = long_alloc(asize + bsize);
     if (ret == NULL) goto fail;
-#ifdef Py_DEBUG
-    /* Fill with trash, to catch reference to uninitialized digits. */
-    memset(ret->long_value.ob_digit, 0xDF, _PyLong_DigitCount(ret) * sizeof(digit));
-#endif
 
     /* 2. t1 <- ah*bh, and copy into high digits of result. */
     if ((t1 = k_mul(ah, bh)) == NULL) goto fail;
@@ -5636,6 +5637,12 @@ long_bitwise(PyLongObject *a,
         Py_UNREACHABLE();
     }
 
+    if ((size_z + negz) == 0) {
+        Py_XDECREF(new_a);
+        Py_XDECREF(new_b);
+        return get_small_int(0);
+    }
+
     /* We allow an extra digit if z is negative, to make sure that
        the final two's complement of z doesn't overflow. */
     z = long_alloc(size_z + negz);
@@ -6959,6 +6966,28 @@ PyLongWriter_Finish(PyLongWriter *writer)
     PyLongObject *obj = (PyLongObject *)writer;
     assert(Py_REFCNT(obj) == 1);
 
+#ifdef Py_DEBUG
+    // gh-147988: Detect uninitialized digits: long_alloc() fills digits with
+    // 0xFF byte pattern. It's posssible because PyLong_BASE is smaller than
+    // the maximum value of the C digit type (uint32_t or unsigned short):
+    // most significan bits are unused by the API.
+    Py_ssize_t ndigits = _PyLong_DigitCount(obj);
+    if (ndigits == 0) {
+        // Check ob_digit[0] digit for the number zero
+        ndigits = 1;
+    }
+    for (Py_ssize_t i = 0; i < ndigits; i++) {
+        digit d = obj->long_value.ob_digit[i];
+        if (d & ~(digit)PyLong_MASK) {
+            Py_DECREF(obj);
+            PyErr_Format(PyExc_SystemError,
+                         "PyLongWriter_Finish: digit %zd is uninitialized",
+                         i);
+            return NULL;
+        }
+    }
+#endif
+
     // Normalize and get singleton if possible
     obj = maybe_small_long(long_normalize(obj));