gh-143004: Fix UAF in _collections Counter update fast path

[Kaushalt2004]

Problem
_collections._count_elements has a fast-path for dict that reads oldval via _PyDict_GetItem_KnownHash() (borrowed reference) and then calls PyNumber_Add(oldval, one). PyNumber_Add() can run arbitrary Python code (e.g. add), allowing re-entrant mutation such as Counter.clear(). That can drop the last reference to oldval while the C code still holds a borrowed pointer, resulting in a use-after-free (ASAN).

Reproducer

Fix
In the fast dict path, keep oldval alive across PyNumber_Add() by temporarily owning a reference:

Py_INCREF(oldval); newval = PyNumber_Add(oldval, one); Py_DECREF(oldval);
This prevents oldval from being freed if user code re-enters and clears/mutates the mapping during the add.

Changes

_collectionsmodule.c: INCREF/DECREF oldval around PyNumber_Add() in the fast path.
test_collections.py: add regression test test_update_reentrant_add_clears_counter.
Tests

./python -m test test_collections -k reentrant_add -v

• Issue: Use-after-free in Counter.update via re-entrant __add__ #143004

[bedevere-app]

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.

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[StanFromIreland]

Please create an issue and add a blurb.

[skirpichev]

Please create an issue

@StanFromIreland, it's #143004.

@Kaushalt2004, while technically this seems to be correct, this is not something that solves real-world problem. Could you please provide benchmarks to show how this will impact more typical use-cases for Counter()?

[Kaushalt2004]

I ran a small microbenchmark to estimate the impact of the Py_INCREF(oldval) / Py_DECREF(oldval) pair added around PyNumber_Add() in the dict fast-path.

Setup

Windows x64
Built both revisions with the same compiler/toolchain to keep the comparison fair.
Note: my VS install is VS 2026; build.bat expects older MSBuild toolset targets (v143). I built both revisions via MSBuild using PlatformToolset=v145 (same for “before” and “after”).
How

Before: origin/main (b8d3fdd)
After: this PR branch (ce88c7e)
Build + run commands (same for both revisions; only git checkout changes)

subst X: "C:\Users\...\New folder (12)"
cd /d X:\cpython
MSBuild pcbuild.proj /t:Build /m /nologo /v:m /clp:summary /p:Configuration=Release /p:Platform=x64 /p:PlatformToolset=v145 /p:IncludeExternals=true /p:IncludeCTypes=true /p:IncludeSSL=true /p:IncludeTkinter=false
PCbuild\amd64\python.exe Tools\scripts\bench_counter_update.py --repeats 25 --inner-loops 50 --n-keys 1000 --n-elems 200000
Results (ns/call; lower is better)

update(unique) from empty: 26218.0 → 26438.0 (+0.84%)
update(dupes) from empty: 4520476.0 → 4502494.0 (-0.40%)
update(dupes) preseeded (hits the INCREF/DECREF path every time): 4474634.0 → 4499386.0 (+0.55%)
update(string dupes) empty: 6083882.0 → 6028120.0 (-0.92%)
Conclusion

Deltas are within ~±1% on this machine; no meaningful regression observed, including the “preseeded” case that exercises the new INCREF/DECREF on every update.
If you want it even tighter (1–2 paragraphs), tell me whether you prefer “minimal” or “detailed,” and I’ll rewrite it.I ran a small microbenchmark to estimate the impact of the Py_INCREF(oldval) / Py_DECREF(oldval) pair added around PyNumber_Add() in the dict fast-path.

Setup

Windows x64
Built both revisions with the same compiler/toolchain to keep the comparison fair.
Note: my VS install is VS 2026; build.bat expects older MSBuild toolset targets (v143). I built both revisions via MSBuild using PlatformToolset=v145 (same for “before” and “after”).
How

Before: origin/main (b8d3fdd)
After: this PR branch (ce88c7e)
Build + run commands (same for both revisions; only git checkout changes)

subst X: "C:\Users\...\New folder (12)"
cd /d X:\cpython
MSBuild pcbuild.proj /t:Build /m /nologo /v:m /clp:summary /p:Configuration=Release /p:Platform=x64 /p:PlatformToolset=v145 /p:IncludeExternals=true /p:IncludeCTypes=true /p:IncludeSSL=true /p:IncludeTkinter=false
PCbuild\amd64\python.exe Tools\scripts\bench_counter_update.py --repeats 25 --inner-loops 50 --n-keys 1000 --n-elems 200000
Results (ns/call; lower is better)

update(unique) from empty: 26218.0 → 26438.0 (+0.84%)
update(dupes) from empty: 4520476.0 → 4502494.0 (-0.40%)
update(dupes) preseeded (hits the INCREF/DECREF path every time): 4474634.0 → 4499386.0 (+0.55%)
update(string dupes) empty: 6083882.0 → 6028120.0 (-0.92%)
Conclusion

Deltas are within ~±1% on this machine; no meaningful regression observed, including the “preseeded” case that exercises the new INCREF/DECREF on every update.

[bedevere-app]

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.

[skirpichev]

I ran a small microbenchmark

I still see no benchmark code.

PS: @Kaushalt2004, do not click Update branch without a good reason because it notifies everyone watching the PR that there are new changes, when there are not, and it uses up limited CI resources.

[Kaushalt2004]

Thanks for the feedback.

I’ve removed the benchmark script from the PR as requested.
I’ve also adjusted the NEWS entry formatting.

Benchmark methodology and results were shared earlier in comments;
they show no meaningful regression (~±1%).

Please let me know if you’d like the benchmark rerun using pyperf
or if further changes are needed.

[Kaushalt2004]

Thanks for the clarification. You’re right that Python guarantees subclass construction, and the test does not demonstrate any loss of subclass identity.
The custom new was added defensively, but since this behavior cannot be concretely shown, it is unnecessary.
I will simplify the test by removing the custom constructor and keep it focused on the re-entrant mutation via add.

[serhiy-storchaka]

LGTM. 👍

[miss-islington-app]

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

[bedevere-app]

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

[bedevere-app]

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