Skip to content

Commit e0c28e2

Browse files
pablogsalStanFromIrelandvstinner
authored
[3.14] Add MSan to CI (GH-158625) (#158834)
* Run a job that the test suite with MSan to the CI (#158625) * Run the test suite with MSan in CI * Additional fixes * Add `_Py_MSAN_UNPOISON_STRING` * Apply Victor's suggestions Co-authored-by: Victor Stinner <victor.stinner@gmail.com> * Apply Victor's suggestions Co-authored-by: Victor Stinner <victor.stinner@gmail.com> --------- Co-authored-by: Victor Stinner <victor.stinner@gmail.com> (cherry picked from commit b93fb19) * Mark bytes returned by getrandom as initialized for MSan * Handle invalid non-ASCII struct formats in the fuzz harness --------- Co-authored-by: Stan Ulbrych <stan@python.org> Co-authored-by: Victor Stinner <victor.stinner@gmail.com>
1 parent 4753622 commit e0c28e2

13 files changed

Lines changed: 56 additions & 10 deletions

File tree

‎.github/workflows/build.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,9 @@ jobs:
589589
- check-name: Undefined behavior
590590
sanitizer: UBSan
591591
free-threading: false
592+
- check-name: Memory
593+
sanitizer: MSan
594+
free-threading: false
592595
uses: ./.github/workflows/reusable-san.yml
593596
with:
594597
sanitizer: ${{ matrix.sanitizer }}

‎.github/workflows/reusable-san.yml‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,26 @@ jobs:
6767
|| ''
6868
}}
6969
- name: UBSan option setup
70-
if: inputs.sanitizer != 'TSan'
70+
if: inputs.sanitizer == 'UBSan'
7171
run: >-
7272
echo
7373
"UBSAN_OPTIONS=${SAN_LOG_OPTION}"
7474
>> "$GITHUB_ENV"
7575
env:
7676
SAN_LOG_OPTION: log_path=${{ github.workspace }}/san_log
77+
- name: MSan option setup
78+
if: inputs.sanitizer == 'MSan'
79+
run: |
80+
echo "MSAN_OPTIONS=${SAN_LOG_OPTION} allocator_may_return_null=1 handle_segv=0" >> "$GITHUB_ENV"
81+
# MSan reports false positives for memory initialized by libraries
82+
# that are not built with MSan, so disable modules that use them.
83+
{
84+
echo '*disabled*'
85+
echo '_bz2 _ctypes _curses _curses_panel _dbm _decimal _gdbm _hashlib'
86+
echo '_lzma _sqlite3 _ssl _tkinter _uuid _zstd readline zlib'
87+
} > Modules/Setup.local
88+
env:
89+
SAN_LOG_OPTION: log_path=${{ github.workspace }}/san_log
7790
- name: Add ccache to PATH
7891
run: |
7992
echo "PATH=/usr/lib/ccache:$PATH" >> "$GITHUB_ENV"
@@ -98,6 +111,8 @@ jobs:
98111
# gh-157958: -O2 instead of the pydebug default -Og to avoid a clang 21
99112
# compile-time blowup on some interpreter files.
100113
# (https://github.com/llvm/llvm-project/issues/179695)
114+
# MSan uses --with-assertions instead of --with-pydebug because its
115+
# hooks on the Python memory allocators hide uninitialized reads.
101116
- name: Configure CPython
102117
run: >-
103118
./configure
@@ -106,9 +121,11 @@ jobs:
106121
${{
107122
inputs.sanitizer == 'TSan'
108123
&& '--with-thread-sanitizer'
124+
|| inputs.sanitizer == 'MSan'
125+
&& '--with-memory-sanitizer'
109126
|| '--with-undefined-behavior-sanitizer'
110127
}}
111-
--with-pydebug
128+
${{ inputs.sanitizer == 'MSan' && '--with-assertions' || '--with-pydebug' }}
112129
${{ inputs.sanitizer == 'TSan' && '--with-openssl="$OPENSSL_DIR" --with-openssl-rpath=auto' || '' }}
113130
${{ inputs.free-threading && '--disable-gil' || '' }}
114131
- name: Build CPython

‎Doc/using/configure.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,10 @@ Debug options
920920

921921
Enable MemorySanitizer allocation error detector, ``msan`` (default is no).
922922

923+
MSan reports false positives for memory initialized by libraries that are
924+
not built with MSan, so either build all dependencies with MSan or disable
925+
the extension modules that use them in :file:`Modules/Setup.local`.
926+
923927
.. versionadded:: 3.6
924928

925929
.. option:: --with-undefined-behavior-sanitizer

‎Include/pyport.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,8 @@ extern "C" {
577577
# if !defined(_Py_MEMORY_SANITIZER)
578578
# define _Py_MEMORY_SANITIZER
579579
# define _Py_NO_SANITIZE_MEMORY __attribute__((no_sanitize_memory))
580+
# define _Py_MSAN_UNPOISON(PTR, SIZE) (__msan_unpoison(PTR, SIZE))
581+
# define _Py_MSAN_UNPOISON_STRING(STR) (__msan_unpoison_string(STR))
580582
# endif
581583
# endif
582584
# if __has_feature(address_sanitizer)
@@ -615,6 +617,12 @@ extern "C" {
615617
#ifndef _Py_NO_SANITIZE_MEMORY
616618
# define _Py_NO_SANITIZE_MEMORY
617619
#endif
620+
#ifndef _Py_MSAN_UNPOISON
621+
# define _Py_MSAN_UNPOISON(PTR, SIZE)
622+
#endif
623+
#ifndef _Py_MSAN_UNPOISON_STRING
624+
# define _Py_MSAN_UNPOISON_STRING(STR)
625+
#endif
618626

619627
/* AIX has __bool__ redefined in it's system header file. */
620628
#if defined(_AIX) && defined(__bool__)

‎Lib/test/test_faulthandler.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@
3434

3535

3636
def skip_if_sanitizer_signal(signame):
37-
return support.skip_if_sanitizer(f"TSAN/UBSan itercepts {signame}",
38-
thread=True, ub=True)
37+
return support.skip_if_sanitizer(f"TSan/UBSan/MSan intercepts {signame}",
38+
thread=True, ub=True, memory=True)
3939

4040

4141
def expected_traceback(lineno1, lineno2, header, min_count=1):

‎Lib/test/test_xxtestfuzz.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ def test_sample_input_smoke_test(self):
1818
_xxtestfuzz.run(b"1")
1919
_xxtestfuzz.run(b"AAAAAAA")
2020
_xxtestfuzz.run(b"AAAAAA\0")
21+
_xxtestfuzz.run(b"\xff\0")
2122

2223

2324
if __name__ == "__main__":

‎Modules/_testinternalcapi.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,8 +1489,8 @@ check_pyobject_forbidden_bytes_is_freed(PyObject *self,
14891489
static PyObject *
14901490
check_pyobject_freed_is_freed(PyObject *self, PyObject *Py_UNUSED(args))
14911491
{
1492-
/* ASan or TSan would report an use-after-free error */
1493-
#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER)
1492+
/* ASan, MSan or TSan would report an error. */
1493+
#if defined(_Py_ADDRESS_SANITIZER) || defined(_Py_THREAD_SANITIZER) || defined(_Py_MEMORY_SANITIZER)
14941494
Py_RETURN_NONE;
14951495
#else
14961496
PyObject *op = PyObject_CallNoArgs((PyObject *)&PyBaseObject_Type);

‎Modules/_xxtestfuzz/fuzzer.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ static int fuzz_struct_unpack(const char* data, size_t size) {
133133
if (unpacked == NULL && PyErr_ExceptionMatches(PyExc_SystemError)) {
134134
PyErr_Clear();
135135
}
136+
/* Ignore any ValueError, these are triggered by non-ASCII format. */
137+
if (unpacked == NULL && PyErr_ExceptionMatches(PyExc_ValueError)) {
138+
PyErr_Clear();
139+
}
136140
/* Ignore any struct.error exceptions, these can be caused by invalid
137141
formats or incomplete buffers both of which are common. */
138142
if (unpacked == NULL && PyErr_ExceptionMatches(struct_error)) {

‎Modules/posixmodule.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9660,6 +9660,7 @@ os_getlogin_impl(PyObject *module)
96609660
errno = old_errno;
96619661
}
96629662
else {
9663+
_Py_MSAN_UNPOISON(name, sizeof(name));
96639664
result = PyUnicode_DecodeFSDefault(name);
96649665
}
96659666
#else
@@ -16823,6 +16824,8 @@ os_getrandom_impl(PyObject *module, Py_ssize_t size, int flags)
1682316824
goto error;
1682416825
}
1682516826

16827+
_Py_MSAN_UNPOISON(PyBytes_AS_STRING(bytes), n);
16828+
1682616829
if (n != size) {
1682716830
_PyBytes_Resize(&bytes, n);
1682816831
}

‎Modules/socketmodule.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,9 @@ set_herror(socket_state *state, int h_error)
752752
PyObject *v;
753753

754754
#ifdef HAVE_HSTRERROR
755-
v = Py_BuildValue("(iN)", h_error, decode_error_message(hstrerror(h_error)));
755+
const char *errmsg = hstrerror(h_error);
756+
_Py_MSAN_UNPOISON_STRING(errmsg);
757+
v = Py_BuildValue("(iN)", h_error, decode_error_message(errmsg));
756758
#else
757759
v = Py_BuildValue("(is)", h_error, "host not found");
758760
#endif
@@ -779,7 +781,9 @@ set_gaierror(socket_state *state, int error)
779781
#endif
780782

781783
#ifdef HAVE_GAI_STRERROR
782-
v = Py_BuildValue("(iN)", error, decode_error_message(gai_strerror(error)));
784+
const char *errmsg = gai_strerror(error);
785+
_Py_MSAN_UNPOISON_STRING(errmsg);
786+
v = Py_BuildValue("(iN)", error, decode_error_message(errmsg));
783787
#else
784788
v = Py_BuildValue("(is)", error, "getaddrinfo failed");
785789
#endif
@@ -6408,6 +6412,7 @@ socket_getservbyport(PyObject *self, PyObject *args)
64086412
PyErr_SetString(PyExc_OSError, "port/proto not found");
64096413
return NULL;
64106414
}
6415+
_Py_MSAN_UNPOISON_STRING(sp->s_name);
64116416
return PyUnicode_FromString(sp->s_name);
64126417
}
64136418

0 commit comments

Comments
 (0)