Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-89455: Fix an uninitialized bool in exception print context. #91466

Merged
merged 3 commits into from
Apr 14, 2022

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented Apr 12, 2022

Fix an uninitialized bool in exception print context.

struct exception_print_context.need_close was uninitialized.

Found by oss-fuzz in a test case running under the undefined behavior sanitizer.

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46618
https://oss-fuzz.com/testcase-detail/6217746058182656

Python/pythonrun.c:1241:28: runtime error: load of value 253, which is not a valid value for type 'bool'
    #0 0xbf2203 in print_chained cpython3/Python/pythonrun.c:1241:28
    #1 0xbea4bb in print_exception_cause_and_context cpython3/Python/pythonrun.c:1320:19
    #2 0xbea4bb in print_exception_recursive cpython3/Python/pythonrun.c:1470:13
    #3 0xbe9e39 in _PyErr_Display cpython3/Python/pythonrun.c:1517:9

Pretty obvious what the ommission was upon code inspection.

@cpython-cla-bot
Copy link

Commit authors are required to sign the Contributor License Agreement.
CLA not signed

@gpshead
Copy link
Member Author

gpshead commented Apr 12, 2022

Commit authors are required to sign the Contributor License Agreement.

The new CLA checker is broken and won't accept my current commits or even my attempt to re-sign the CLA via the bot here because of the way my arbitrary unverified text string git "email" field is structured in many of my local git config files. I'll keep this PR open as an example until that can be resolved.

Copy link
Member

@iritkatriel iritkatriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is initialised to false in line 1379, and the value is only used further down in the same function, so I don't think there's an actual bug (phew).

Yes, let's initialise it here too.

`struct exception_print_context.need_close` was uninitialized.

Found by oss-fuzz in a test case running under the undefined behavior sanitizer.

https://oss-fuzz.com/testcase-detail/6217746058182656

```
Python/pythonrun.c:1241:28: runtime error: load of value 253, which is not a valid value for type 'bool'
    #0 0xbf2203 in print_chained cpython3/Python/pythonrun.c:1241:28
    #1 0xbea4bb in print_exception_cause_and_context cpython3/Python/pythonrun.c:1320:19
    #2 0xbea4bb in print_exception_recursive cpython3/Python/pythonrun.c:1470:13
    #3 0xbe9e39 in _PyErr_Display cpython3/Python/pythonrun.c:1517:9
```

Pretty obvious what the ommission was upon code inspection.
@gpshead gpshead force-pushed the bpo-45292-uninitialized_need_close branch from dc7f776 to a012f40 Compare April 13, 2022 00:47
@gpshead gpshead closed this Apr 13, 2022
@gpshead gpshead reopened this Apr 13, 2022
also see if a new commit re-triggers the CLA checker.
@gpshead gpshead marked this pull request as ready for review April 14, 2022 18:22
@gpshead
Copy link
Member Author

gpshead commented Apr 14, 2022

This is a real bug, the codepath in the traceback listed above used the value without it being initialized. Thus a sanitizer finding it. I don't think it had noticable behavioral impact though.

@gpshead gpshead merged commit 861974b into python:main Apr 14, 2022
@gpshead gpshead deleted the bpo-45292-uninitialized_need_close branch April 14, 2022 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants