Skip to content

gh-134070: Prevent out-of-bounds read in mi_clz32 and mi_ctz32 #134149

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
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

gh-134070: Prevent out-of-bounds read in mi_clz32 and mi_ctz32 #134149

wants to merge 6 commits into from
+5 −2

Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant

Conversation

vedant713
Copy link

Fixes GH-134070

Summary:
This patch adds a masking operation to the index calculation in mi_ctz32() and mi_clz32() functions to prevent out-of-bounds access on 64-bit systems, matching the fix from upstream mimalloc.

Why:
On systems where unsigned long is 64-bit, the multiplication may produce a value >31, causing a read outside the array bounds.

@bedevere-app
Copy link

bedevere-app bot commented May 17, 2025

Most changes to Python blurb command-line tool.

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

@ZeroIntensity ZeroIntensity changed the title Fix: Prevent out-of-bounds read in mi_clz32 and mi_ctz32 (#134070) gh-134070: Prevent out-of-bounds read in mi_clz32 and mi_ctz32 (#134070) May 17, 2025
@ZeroIntensity ZeroIntensity requested a review from colesbury May 17, 2025 18:51
@bedevere-app bedevere-app bot mentioned this pull request May 17, 2025
Open
@ZeroIntensity ZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels May 17, 2025
@ZeroIntensity ZeroIntensity changed the title gh-134070: Prevent out-of-bounds read in mi_clz32 and mi_ctz32 (#134070) gh-134070: Prevent out-of-bounds read in mi_clz32 and mi_ctz32 May 17, 2025
ZeroIntensity
ZeroIntensity reviewed May 17, 2025
View reviewed changes
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Please add a test and blurb.

@collinfunk
Copy link
Contributor

FYI, this code is from https://github.com/microsoft/mimalloc. You might want to send a pull request there too.

@picnixz
Copy link
Member

picnixz commented May 17, 2025

It's said to be fixed in microsoft/mimalloc@ed31847.

vedant713 added a commit to vedant713/cpython that referenced this pull request May 17, 2025
@vedant713
p…
960e266 
…ythonGH-134149)
@bedevere-app
Copy link

bedevere-app bot commented May 17, 2025

Most changes to Python blurb command-line tool.

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

@picnixz
Copy link
Member

picnixz commented May 17, 2025

Please, stop updating two PRs at the same time. See

@picnixz picnixz mentioned this pull request May 17, 2025
Closed
@vedant713
Copy link
Author

I'm trying to add via heroku app but giving server error
bedevere/news
bedevere/news — No news entry in Misc/NEWS.d/next/ or "skip news" label found

@picnixz
Copy link
Member

picnixz commented May 17, 2025 •
edited
Loading

I never used the app, so I'd suggest using the command-line instead:

python -m pip install blurb
python -m blurb
git commit -m 'blurb'

@picnixz picnixz marked this pull request as draft May 17, 2025 20:33
@vedant713 vedant713 marked this pull request as ready for review May 17, 2025 21:52
@collinfunk
Copy link
Contributor

@picnixz Thanks for checking. Somehow I missed that commit in mimalloc.

@colesbury
Copy link
Contributor

I think we should copy-paste the change from upstream instead of doing a similar (but not identical) fix here.

In other words, copy this code from upstream (from the fix commit) into our mimalloc/internal.h file.

https://github.com/microsoft/mimalloc/blob/ed318471126918fce7caf0001cf1e0c78f95173e/include/mimalloc/internal.h#L875-L932

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

@ZeroIntensity ZeroIntensity

@colesbury colesbury

Assignees
No one assigned
Labels
awaiting review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Out-of-bounds read in integrated mimalloc (fixed upstream)

None yet

Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant

5 participants
@vedant713 @collinfunk @picnixz @colesbury @ZeroIntensity

Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant