FazBrowse GitHub Viewer | Trending | | Home

Tools: [Download Repo ZIP] [Original HTTPS Page]

gh-112127: Fix use-after-free in atexit.unregister() by benjaminJohnson2204 · Pull Request #114092 · python/cpython · GitHub

gh-112127: Fix use-after-free in atexit.unregister() - #114092

Merged

serhiy-storchaka merged 12 commits into

python:mainfrom

benjaminJohnson2204:gh-112127

Dec 17, 2025

benjaminJohnson2204 commented Jan 15, 2024 •
edited by bedevere-app Bot
Loading

Copy link Copy Markdown

Contributor

This PR fixes a use-after-free error when a callback registered with atexit.register() calls atexit.unregister() or atexit._clear() in its __eq__ method, which is called by the atexit_register() C API.

I fixed the issue by increasing the refcounts of both arguments to the __eq__ check before the equality check, and decrementing them afterward.

I also added additional unit tests to test that these cases do not cause crashes.

I don't think this needs a news entry.

Issue: #112127

Issue: Use-after-free in unregister() of atexit module #112127

Fix use-after-free in atexit.unregister()

6c8db8a

ghost commented Jan 15, 2024 •
edited by ghost
Loading

Copy link Copy Markdown

All commit authors signed the Contributor License Agreement.

bedevere-app Bot commented Jan 15, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app Bot added the awaiting review label Jan 15, 2024

bedevere-app Bot mentioned this pull request Jan 15, 2024

Use-after-free in unregister() of atexit module #112127

Closed

Merge branch 'main' into pythongh-112127

0623581

bedevere-app Bot commented Jan 15, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

sobolevn reviewed Feb 12, 2024

View reviewed changes

sobolevn left a comment

Copy link Copy Markdown

Member

Thanks! I have several comments.

Comment thread Lib/test/_test_atexit.py Outdated

Comment thread Lib/test/_test_atexit.py Outdated

Comment thread Lib/test/_test_atexit.py Outdated

Comment thread Modules/atexitmodule.c Outdated

Merge branch 'main' into pythongh-112127

8e6d5c6

bedevere-app Bot commented Feb 13, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Address code review comments in atexit tests

075c49f

bedevere-app Bot commented Feb 13, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Fix linter issue

5b022fc

bedevere-app Bot commented Feb 13, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

benjaminJohnson2204 requested a review from sobolevn February 13, 2024 18:52

Merge branch 'main' into pythongh-112127

1148d80

bedevere-app Bot commented Jul 17, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

serhiy-storchaka reviewed Jul 17, 2024

View reviewed changes

Comment thread Modules/atexitmodule.c Outdated

Comment thread Lib/test/_test_atexit.py Outdated

Comment thread Lib/test/_test_atexit.py Outdated

Address PR comments

9056824

bedevere-app Bot commented Jul 18, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

benjaminJohnson2204 commented Jul 18, 2024

Copy link Copy Markdown

Contributor Author

Thanks for the feedback @serhiy-storchaka ! I updated my PR based on your comments, could you take another look when you get a chance please?

benjaminJohnson2204 requested a review from serhiy-storchaka July 18, 2024 02:57

Remove trailing whitespace

0775ca8

bedevere-app Bot commented Jul 18, 2024

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

serhiy-storchaka approved these changes Jul 18, 2024

View reviewed changes

serhiy-storchaka left a comment

Copy link Copy Markdown

Member

LGTM.

bedevere-app Bot added awaiting merge and removed awaiting review labels Jul 18, 2024

kumaraditya303 requested changes Jul 3, 2025

View reviewed changes

kumaraditya303 left a comment

Copy link Copy Markdown

Contributor

There are merge conflicts

bedevere-app Bot removed the awaiting merge label Jul 3, 2025

bedevere-app Bot commented Jul 3, 2025

Copy link Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

serhiy-storchaka commented Dec 17, 2025

Copy link Copy Markdown

Member

There is yet one issue here. Even if it no longer crashes, it can unregister wrong function if other function was unregistered during execution of __eq__().

After finding the callback, we should check that the tuple at the current position is the same, and if it is not, iterate back until we find it (items can only be removed, not inserted before the current position, unless we play with gc.get_objects()).

serhiy-storchaka self-requested a review December 17, 2025 09:45

serhiy-storchaka reviewed Dec 17, 2025

View reviewed changes

serhiy-storchaka left a comment

Copy link Copy Markdown

Member

Also, currently the test is passed with unpatched code.

Comment thread Lib/test/_test_atexit.py Outdated

    
                              cnt += 1

                              if cnt == 1:

                                  self.action(o)

                              return self.eq_ret_val(o)

serhiy-storchaka Dec 17, 2025

Copy link Copy Markdown

Member

self.eq_ret_val is not callable.

serhiy-storchaka self-assigned this Dec 17, 2025

Fix tests.

143fd96

bedevere-app Bot commented Dec 17, 2025

Copy link Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

serhiy-storchaka added 2 commits December 17, 2025 14:41

Add a NEWS entry.

99e35c9

Fix use-after-free issue in atexit.unregister

7ec0097

kumaraditya303 approved these changes Dec 17, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting changes labels Dec 17, 2025

serhiy-storchaka merged commit 2b466c4 into python:main Dec 17, 2025

49 of 50 checks passed

bedevere-app Bot removed the awaiting merge label Dec 17, 2025

serhiy-storchaka removed their assignment Dec 17, 2025

serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Dec 17, 2025

miss-islington-app Bot commented Dec 17, 2025

Copy link Copy Markdown

Thanks @benjaminJohnson2204 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app Bot commented Dec 17, 2025

Copy link Copy Markdown

Thanks @benjaminJohnson2204 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington-app Bot commented Dec 17, 2025

Copy link Copy Markdown

Sorry, @benjaminJohnson2204 and @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 2b466c47c333106dc9522ab77898e6972e25a2c6 3.13

miss-islington-app Bot assigned serhiy-storchaka Dec 17, 2025

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 17, 2025

pythongh-112127 : Fix possible use-after-free in atexit.unregister() (p…

04b11aa

…ythonGH-114092)

(cherry picked from commit 2b466c4)

Co-authored-by: Benjamin Johnson <ben332004@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-app Bot commented Dec 17, 2025

Copy link Copy Markdown

GH-142878 is a backport of this pull request to the 3.14 branch.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Dec 17, 2025

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Dec 17, 2025

[3.13] pythongh-112127 : Fix possible use-after-free in atexit.unregis…

ed7bea4

…ter() (pythonGH-114092)

(cherry picked from commit 2b466c4)

Co-authored-by: Benjamin Johnson <ben332004@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

bedevere-app Bot commented Dec 17, 2025

Copy link Copy Markdown

GH-142880 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Dec 17, 2025

kumaraditya303 pushed a commit that referenced this pull request Dec 17, 2025

[3.14] gh-112127 : Fix possible use-after-free in atexit.unregister() (G…

8c92d66

…H-114092) (#142878)

gh-112127: Fix possible use-after-free in atexit.unregister() (GH-114092)
(cherry picked from commit 2b466c4)

Co-authored-by: Benjamin Johnson <ben332004@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Dec 17, 2025

[3.13] pythongh-112127 : Fix possible use-after-free in atexit.unregis…

3a92ca5

…ter() (pythonGH-114092)

(cherry picked from commit 2b466c4)

Co-authored-by: Benjamin Johnson <ben332004@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

serhiy-storchaka added a commit that referenced this pull request Dec 17, 2025

[3.13] gh-112127 : Fix possible use-after-free in atexit.unregister() (G…

08d665d

…H-114092) (GH-142880)

(cherry picked from commit 2b466c4)

Co-authored-by: Benjamin Johnson <ben332004@gmail.com>

serhiy-storchaka removed their assignment Jul 15, 2026

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters

Show hidden characters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

serhiy-storchaka approved these changes

kumaraditya303 approved these changes

Awaiting requested review from sobolevn

Assignees

No one assigned

Labels

None yet

Projects

Milestone

No milestone

Back | FazBrowse Home | New Git URL