classification
Title: Py_BuildValue("(s#O)", ...) segfaults if entered with exception raised
Type: crash Stage: resolved
Components: Extension Modules Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: cstratak, danielen, doko, lukasz.langa, serhiy.storchaka, steve.dower
Priority: release blocker Keywords: 3.8regression, patch

Created on 2019-11-26 06:27 by danielen, last changed 2020-03-02 19:44 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
module.c danielen, 2019-11-26 06:27
Pull Requests
URL Status Linked Edit
PR 18656 merged serhiy.storchaka, 2020-02-25 18:13
PR 18732 merged serhiy.storchaka, 2020-03-02 06:46
Messages (12)
msg357485 - (view) Author: (danielen) Date: 2019-11-26 06:27
The following code results in a segmentation fault in CPython 3.8 while executes fines (raises a SystemError) in CPython 3.7.

PyObject* debug(PyObject *self, PyObject *args)
{
        const char * debug = "debug";
        PyErr_SetString(PyExc_ValueError, "debug!");
        return Py_BuildValue("(s#O)", debug, strlen(debug), Py_None);
}

It seems necessary for the format string to contain both an instance of "s#" and "O" to trigger the bug.

I'm attaching a C module that reproduces the problem.
msg357504 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2019-11-26 16:12
seen with the current 3.8 branch.
msg357512 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-26 17:05
Does it depend on whether PY_SSIZE_T_CLEAN is defined?
msg357514 - (view) Author: (danielen) Date: 2019-11-26 17:10
It is not reproducible with PY_SSIZE_T_CLEAN defined.
msg357554 - (view) Author: (danielen) Date: 2019-11-27 04:38
The problem arises from this code in do_mktuple(), staring at line 394 in modsupport.c:

            if (**p_format == '#') {
                ++*p_format;
                if (flags & FLAG_SIZE_T)
                    n = va_arg(*p_va, Py_ssize_t);
                else {
                    if (PyErr_WarnEx(PyExc_DeprecationWarning,
                                "PY_SSIZE_T_CLEAN will be required for '#' formats", 1)) {
                        return NULL;
                    }
                    n = va_arg(*p_va, int);
                }
            }

If this is entered with an exception raised, PyErr_WarnEx() return NULL, thus this function return NULL without consuming the argument relative to the string length for the "s#" specifier. This argument is then consumed at the next iteration for the "O" specifier, resulting in a segmentation fault when the string length is interpreted as an object pointer.

I don't know what is the best solution: either ignoring the return value of PyErr_WarnEx or swapping the lines from 

                    if (PyErr_WarnEx(PyExc_DeprecationWarning,
                                "PY_SSIZE_T_CLEAN will be required for '#' formats", 1)) {
                        return NULL;
                    }
                    n = va_arg(*p_va, int);


to 

                    n = va_arg(*p_va, int);
                    if (PyErr_WarnEx(PyExc_DeprecationWarning,
                                "PY_SSIZE_T_CLEAN will be required for '#' formats", 1)) {
                        return NULL;
                    }

The handling of the "y#" just below suffers from the same problem.
msg358096 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-12-09 14:18
Note: this is going to miss Python 3.8.1 as I'm releasing 3.8.1rc1 right now.  Please address this before 3.8.2 (due in February).
msg362635 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2020-02-25 12:08
Sadly, this missed the train for 3.8.2 as well.
msg363129 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-02 06:42
New changeset 28d0bcac8b7e6dbd28311f1283dabb6a4d649fcb by Serhiy Storchaka in branch 'master':
bpo-38913: Fix segfault in Py_BuildValue("(s#O)", ...) if entered with exception raised. (GH-18656)
https://github.com/python/cpython/commit/28d0bcac8b7e6dbd28311f1283dabb6a4d649fcb
msg363134 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-02 07:54
New changeset a7b8a969eb3daacb1fcb029a8c5fecb5d09c964b by Serhiy Storchaka in branch '3.8':
[3.8] bpo-38913: Fix segfault in Py_BuildValue("(sGH-O)", ...) if entered with exception raised. (GH-18656). (GH-18732)
https://github.com/python/cpython/commit/a7b8a969eb3daacb1fcb029a8c5fecb5d09c964b
msg363167 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2020-03-02 12:43
After this change, the arm64 buildbots are reporting reference leaks:

1:03:24 load avg: 0.95 Re-running failed tests in verbose mode
1:03:24 load avg: 0.95 Re-running test_capi in verbose mode
test_capi leaked [4, 4, 4] references, sum=12

e.g. https://buildbot.python.org/all/#/builders/563/builds/25
msg363200 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-02 19:28
Actually there is a leak in PyErr_WarnEx().
msg363204 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-02 19:44
Opened issue39831.
History
Date User Action Args
2020-03-02 19:44:34serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg363204
2020-03-02 19:28:42serhiy.storchakasetmessages: + msg363200
2020-03-02 13:21:22vstinnersetresolution: fixed -> (no value)
2020-03-02 13:01:47serhiy.storchakasetstatus: closed -> open
2020-03-02 12:43:36cstrataksetnosy: + cstratak
messages: + msg363167
2020-03-02 07:55:32serhiy.storchakasetstatus: open -> closed
type: crash
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.9
2020-03-02 07:54:48serhiy.storchakasetmessages: + msg363134
2020-03-02 06:46:51serhiy.storchakasetpull_requests: + pull_request18089
2020-03-02 06:42:45serhiy.storchakasetmessages: + msg363129
2020-02-25 18:13:27serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request18017
2020-02-25 12:26:58serhiy.storchakasetnosy: + serhiy.storchaka
2020-02-25 12:08:10lukasz.langasetmessages: + msg362635
2019-12-09 14:18:28lukasz.langasetmessages: + msg358096
2019-11-27 04:38:55danielensetmessages: + msg357554
2019-11-26 17:10:31danielensetmessages: + msg357514
2019-11-26 17:05:01steve.dowersetnosy: + steve.dower
messages: + msg357512
2019-11-26 16:12:20dokosetpriority: normal -> release blocker

nosy: + lukasz.langa, doko
messages: + msg357504

keywords: + 3.8regression
2019-11-26 06:27:21danielencreate