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.

Sign up for GitHub

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

Jump to bottom

bpo-45826: Fix a crash in suggestions.c by checking for traceback is None #29590

Merged
merged 5 commits into from
Nov 17, 2021
Merged

bpo-45826: Fix a crash in suggestions.c by checking for traceback is None #29590

merged 5 commits into from
Nov 17, 2021

Conversation

sweeneyde
Copy link
Member

@sweeneyde sweeneyde commented Nov 17, 2021 •
edited by bedevere-bot
Loading

@sweeneyde sweeneyde added needs backport to 3.10 only security fixes type-bug An unexpected behavior, bug, or error labels Nov 17, 2021
pablogsal
pablogsal reviewed Nov 17, 2021
View reviewed changes
Python/suggestions.c
@@ -202,13 +202,22 @@ offer_suggestions_for_name_error(PyNameErrorObject *exc)
PyTracebackObject *traceback = (PyTracebackObject *) exc->traceback; // borrowed reference
// Abort if we don't have a variable name or we have an invalid one
// or if we don't have a traceback to work with
if (name == NULL || traceback == NULL || !PyUnicode_CheckExact(name)) {
if (name == NULL || !PyUnicode_CheckExact(name) ||
Copy link
Member

@pablogsal pablogsal Nov 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of checking for None, we can check if the traceback object parent class is the traceback type (.PyTraceBack_Type) . This way we also stop segfaults if someone placed any other object there

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't tb_next is supposed to be None or a Traceback? Regardless, I agree that it is more clear to just check for type.

@ambv ambv merged commit 5d90c46 into python:main Nov 17, 2021
@miss-islington
Copy link
Contributor

Thanks @sweeneyde for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @sweeneyde and @ambv, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 5d90c467c02ffefdb13c1abc83a171db1a99ffad 3.10

ambv pushed a commit to ambv/cpython that referenced this pull request Nov 17, 2021
@sweeneyde @ambv
[3.10] bpo-45826: Fix a crash in suggestions.c by checking for `trace…
7e9e39d 
…back is None` (pythonGH-29590)

(cherry picked from commit 5d90c46)

Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
@bedevere-bot
Copy link

GH-29602 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Nov 17, 2021
ambv added a commit that referenced this pull request Nov 18, 2021
@ambv @sweeneyde
[3.10] bpo-45826: Fix a crash in suggestions.c by checking for `trace…
8eabe60 
…back is None` (GH-29590) (GH-29602)

(cherry picked from commit 5d90c46)

Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
@sweeneyde @remykarem
bpo-45826: Fix a crash in suggestions.c by checking for `traceback is…
5fdb408 
… None` (pythonGH-29590)
@sweeneyde sweeneyde deleted the suggest_crash branch December 19, 2021 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablogsal pablogsal pablogsal left review comments

Assignees

@ambv ambv

Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sweeneyde @miss-islington @bedevere-bot @pablogsal @ambv @the-knights-who-say-ni