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

gh-103295: fix stack overwrite on 32-bit in perf map test harness #104811

Merged
merged 1 commit into from
May 23, 2023
Merged

gh-103295: fix stack overwrite on 32-bit in perf map test harness #104811

merged 1 commit into from
May 23, 2023

Conversation

carljm
Copy link
Member

@carljm carljm commented May 23, 2023 •
edited by bedevere-bot
Loading

We can't use K format specifier in PyArg_ParseTuple to write to a void *; it always writes 64 bits, so writes too much on 32-bit systems. Use O instead and an explicit call to PyLong_AsVoidPtr to get the pointer value.

Also fix the error check so it detects either error code (-1 or -2) and raises a more informative error using errno. I verified this error handling by temporarily changing the perf map file path to a nonexistent directory, and the test raised FileNotFoundError as expected.

Also simplify the return value to just use PyLong_FromLong; we don't need to go through Py_BuildValue for such a simple case.

@bedevere-bot bedevere-bot mentioned this pull request May 23, 2023
Closed
@carljm carljm marked this pull request as ready for review May 23, 2023 19:40
@carljm carljm requested a review from pablogsal May 23, 2023 19:41
@hroncok
Copy link
Contributor

hroncok commented May 23, 2023

A Fedora build with this patch applied on top of 3.12.0b1 tarball.

https://koji.fedoraproject.org/koji/taskinfo?taskID=101500963

It is hopefully set up not to terminate the build immediately when one architecture fails. Let's see.

@carljm carljm added the needs backport to 3.12 bug and security fixes label May 23, 2023
@carljm
Copy link
Member Author

carljm commented May 23, 2023

Looks like the builds all succeeded 🎉

@hroncok
Copy link
Contributor

hroncok commented May 23, 2023

A Fedora build with this patch applied on top of 3.12.0b1 tarball.

https://koji.fedoraproject.org/koji/taskinfo?taskID=101500963

All architectures built, tests have passed.

@carljm
Copy link
Member Author

carljm commented May 23, 2023

I'm going to go ahead and merge this to help unblock the Fedora builds, since this PR only touches test harness and the test signal is all good (including the Fedora build signal on multiple platforms, including 32-bit.)

If there are any concerns with anything here, I'll be happy to do a follow-up PR to address comments.

@carljm carljm merged commit e0b3078 into python:main May 23, 2023
@miss-islington
Copy link
Contributor

Thanks @carljm for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@carljm carljm deleted the fixperfmaptest branch May 23, 2023 22:04
@bedevere-bot
Copy link

GH-104823 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label May 23, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 23, 2023
@carljm @miss-islington
pythongh-103295: fix stack overwrite on 32-bit in perf map test harne…
309d497 
…ss (pythonGH-104811)

(cherry picked from commit e0b3078)

Co-authored-by: Carl Meyer <carl@oddbird.net>
carljm added a commit that referenced this pull request May 23, 2023
@miss-islington @carljm
[3.12] gh-103295: fix stack overwrite on 32-bit in perf map test harn…
22c45c4 
…ess (GH-104811) (#104823)

gh-103295: fix stack overwrite on 32-bit in perf map test harness (GH-104811)
(cherry picked from commit e0b3078)

Co-authored-by: Carl Meyer <carl@oddbird.net>
hrnciar added a commit to fedora-python/cpython that referenced this pull request May 24, 2023
@hrnciar @carljm
00398: fix stack overwrite on 32-bit in perf map test harness (python…
7353ed8 
…GH-104811) (python#104823)

pythongh-103295: fix stack overwrite on 32-bit in perf map test harness (pythonGH-104811)
(cherry picked from commit e0b3078)

Co-authored-by: Carl Meyer <carl@oddbird.net>
hrnciar pushed a commit to fedora-python/cpython that referenced this pull request May 24, 2023
@carljm @hrnciar
00398: fix stack overwrite on 32-bit in perf map test harness (python…
45c1382 
…#104811)
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request May 24, 2023
@facebook-github-bot
User error codes and fix 32-bit system issues
0a7cc0b 
Summary:
Port the latest changes from the [perf-map C-API PR](python/cpython#103546). This includes using error codes instead of exceptions to avoid the need to hold the GIL.

Additionally, there was an issue with Fedora on a 32-bit system that was fixed in this [PR](python/cpython#104811), so I also ported the fix.

Reviewed By: carljm

Differential Revision: D46041553

fbshipit-source-id: 6502189f489ca6cf11949ecd93c044e5ebe4fd2c
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jun 3, 2023
@bmwiedemann
Update python312 to version 3.12.0b1 / rev 2 via SR 1090558
25573f1 
https://build.opensuse.org/request/show/1090558
by user mcepl + dimstar_suse
- Add 00398-fix-stack-overwrite-on-32-bit-in-perf-map-test-harness-gh-104811-104823.patch
  gh#python/cpython#104811
- Refresh all patches
- Update to 3.12.0b1:
  Full changelog can be found here
  https://docs.python.org/dev/whatsnew/changelog.html#python-3-12-0-beta-1
@erlend-aasland erlend-aasland mentioned this pull request Jun 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablogsal pablogsal Awaiting requested review from pablogsal

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@carljm @hroncok @miss-islington @bedevere-bot