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

benjaminJohnson2204 · 2024-01-15T22:06:10Z

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

ghost · 2024-01-15T22:06:13Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2024-01-15T22:06:14Z

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 · 2024-01-15T22:38:09Z

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

Thanks! I have several comments.

Lib/test/_test_atexit.py

Modules/atexitmodule.c

bedevere-app · 2024-02-13T08:32:19Z

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 · 2024-02-13T09:07:26Z

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 · 2024-02-13T16:39:14Z

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 · 2024-07-17T06:27:13Z

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.

Modules/atexitmodule.c

Lib/test/_test_atexit.py

bedevere-app · 2024-07-18T02:56:11Z

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 · 2024-07-18T02:57:26Z

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?

bedevere-app · 2024-07-18T02:58:54Z

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

LGTM.

kumaraditya303

There are merge conflicts

bedevere-app · 2025-07-03T09:42:19Z

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 · 2025-12-17T09:40:21Z

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

Also, currently the test is passed with unpatched code.

serhiy-storchaka · 2025-12-17T09:48:52Z

Lib/test/_test_atexit.py

+                cnt += 1
+                if cnt == 1:
+                    self.action(o)
+                return self.eq_ret_val(o)


self.eq_ret_val is not callable.

bedevere-app · 2025-12-17T12:37:54Z

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.

miss-islington-app · 2025-12-17T15:10:49Z

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 · 2025-12-17T15:10:49Z

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 · 2025-12-17T15:10:52Z

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

…ythonGH-114092) (cherry picked from commit 2b466c4) Co-authored-by: Benjamin Johnson <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

bedevere-app · 2025-12-17T15:11:07Z

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

…ter() (pythonGH-114092) (cherry picked from commit 2b466c4) Co-authored-by: Benjamin Johnson <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

bedevere-app · 2025-12-17T15:17:53Z

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

…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 <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

…ter() (pythonGH-114092) (cherry picked from commit 2b466c4) Co-authored-by: Benjamin Johnson <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

…H-114092) (GH-142880) (cherry picked from commit 2b466c4) Co-authored-by: Benjamin Johnson <[email protected]>

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

6c8db8a

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

sobolevn reviewed Feb 12, 2024

View reviewed changes

Lib/test/_test_atexit.py Outdated Show resolved Hide resolved

Lib/test/_test_atexit.py Outdated Show resolved Hide resolved

Lib/test/_test_atexit.py Outdated Show resolved Hide resolved

Modules/atexitmodule.c Outdated Show resolved Hide resolved

Merge branch 'main' into pythongh-112127

8e6d5c6

Address code review comments in atexit tests

075c49f

Fix linter issue

5b022fc

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

Merge branch 'main' into pythongh-112127

1148d80

serhiy-storchaka reviewed Jul 17, 2024

View reviewed changes

Modules/atexitmodule.c Outdated Show resolved Hide resolved

Lib/test/_test_atexit.py Outdated Show resolved Hide resolved

Lib/test/_test_atexit.py Outdated Show resolved Hide resolved

Address PR comments

9056824

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

Remove trailing whitespace

0775ca8

serhiy-storchaka approved these changes Jul 18, 2024

View reviewed changes

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

kumaraditya303 requested changes Jul 3, 2025

View reviewed changes

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

serhiy-storchaka requested a review from kumaraditya303 December 17, 2025 09:34

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

serhiy-storchaka reviewed Dec 17, 2025

View reviewed changes

serhiy-storchaka self-assigned this Dec 17, 2025

Fix tests.

143fd96

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 assigned serhiy-storchaka Dec 17, 2025

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

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

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 <[email protected]>

Uh oh!

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

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

Uh oh!

Conversation

benjaminJohnson2204 commented Jan 15, 2024 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jan 15, 2024 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app bot commented Jan 15, 2024

Uh oh!

bedevere-app bot commented Jan 15, 2024

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-app bot commented Feb 13, 2024

Uh oh!

bedevere-app bot commented Feb 13, 2024

Uh oh!

bedevere-app bot commented Feb 13, 2024

Uh oh!

bedevere-app bot commented Jul 17, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-app bot commented Jul 18, 2024

Uh oh!

benjaminJohnson2204 commented Jul 18, 2024

Uh oh!

bedevere-app bot commented Jul 18, 2024

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

kumaraditya303 left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Jul 3, 2025

Uh oh!

serhiy-storchaka commented Dec 17, 2025

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Dec 17, 2025

Uh oh!

Uh oh!

miss-islington-app bot commented Dec 17, 2025

Uh oh!

miss-islington-app bot commented Dec 17, 2025

Uh oh!

miss-islington-app bot commented Dec 17, 2025

Uh oh!

bedevere-app bot commented Dec 17, 2025

Uh oh!

bedevere-app bot commented Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benjaminJohnson2204 commented Jan 15, 2024 •

edited by bedevere-app bot

Loading

ghost commented Jan 15, 2024 •

edited by ghost

Loading