Py_BuildValue("(s#O)", ...) segfaults if entered with exception raised

@doko42

BPO	38913
Nosy	@doko42, @ambv, @serhiy-storchaka, @zooba, @stratakis
PRs	bpo-38913: Fix segfault in Py_BuildValue("(s#O)", ...) if entered with exception raised. #18656 [3.8] bpo-38913: Fix segfault in Py_BuildValue("(sGH-O)", ...) if entered with exception raised. (GH-18656). #18732
Files	module.c

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-03-02.19:44:34.948>
created_at = <Date 2019-11-26.06:27:21.676>
labels = ['extension-modules', '3.8', '3.9', 'type-crash', 'release-blocker']
title = 'Py_BuildValue("(s#O)", ...) segfaults if entered with exception raised'
updated_at = <Date 2020-03-02.19:44:34.946>
user = 'https://bugs.python.org/danielen'

bugs.python.org fields:

activity = <Date 2020-03-02.19:44:34.946>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2020-03-02.19:44:34.948>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2019-11-26.06:27:21.676>
creator = 'danielen'
dependencies = []
files = ['48743']
hgrepos = []
issue_num = 38913
keywords = ['patch', '3.8regression']
message_count = 12.0
messages = ['357485', '357504', '357512', '357514', '357554', '358096', '362635', '363129', '363134', '363167', '363200', '363204']
nosy_count = 6.0
nosy_names = ['doko', 'lukasz.langa', 'serhiy.storchaka', 'steve.dower', 'cstratak', 'danielen']
pr_nums = ['18656', '18732']
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue38913'
versions = ['Python 3.8', 'Python 3.9']

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.

seen with the current 3.8 branch.

Does it depend on whether PY_SSIZE_T_CLEAN is defined?

It is not reproducible with PY_SSIZE_T_CLEAN defined.

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.

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).

Sadly, this missed the train for 3.8.2 as well.

New changeset 28d0bca by Serhiy Storchaka in branch 'master':
bpo-38913: Fix segfault in Py_BuildValue("(s#O)", ...) if entered with exception raised. (GH-18656)
28d0bca

New changeset a7b8a96 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)
a7b8a96

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

Actually there is a leak in PyErr_WarnEx().

Opened bpo-39831.

Navigation Menu

Repository navigation

Py_BuildValue("(s#O)", ...) segfaults if entered with exception raised #83094

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Py_BuildValue("(s#O)", ...) segfaults if entered with exception raised #83094

Description

Activity

danielen commented on Nov 26, 2019

doko42 commented on Nov 26, 2019

zooba commented on Nov 26, 2019

danielen commented on Nov 26, 2019

danielen commented on Nov 27, 2019

ambv commented on Dec 9, 2019

ambv commented on Feb 25, 2020

serhiy-storchaka commented on Mar 2, 2020

serhiy-storchaka commented on Mar 2, 2020

stratakis commented on Mar 2, 2020

serhiy-storchaka commented on Mar 2, 2020

serhiy-storchaka commented on Mar 2, 2020

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions