Skip to content

bpo-1635741: port sha3 module to multi-phase init#21855

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
Merged
vstinner merged 10 commits into from
Sep 2, 2020
Merged

bpo-1635741: port sha3 module to multi-phase init #21855

vstinner merged 10 commits into from
Sep 2, 2020
+194 −136

Conversation

@koubaa
Copy link
Contributor

@koubaakoubaa commented Aug 13, 2020•
edited by bedevere-bot
Loading

@koubaakoubaa requested a review from tiran as a code ownerAugust 13, 2020 12:23
@koubaakoubaa mentioned this pull request Aug 13, 2020
Merged
@koubaa
Copy link
ContributorAuthor

koubaa commented Aug 22, 2020

@vstinner@shihai1991 please review

shihai1991
shihai1991 reviewed Aug 22, 2020
View reviewed changes
vstinner
vstinner reviewed Aug 25, 2020
View reviewed changes
vstinner
vstinner reviewed Aug 26, 2020
View reviewed changes
vstinner
vstinner reviewed Aug 27, 2020
View reviewed changes
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

@corona10@shihai1991: The change is mostly LGTM (remaining comments are mostly about minor coding style issues), but I would prefer a second review. Would you mind to review this PR?

shihai1991
shihai1991 reviewed Aug 27, 2020
View reviewed changes
shihai1991
shihai1991 approved these changes Aug 28, 2020
View reviewed changes
Copy link
Member

@shihai1991shihai1991 left a comment

Choose a reason for hiding this comment

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

Thans for update.

vstinner
vstinner reviewed Aug 28, 2020
View reviewed changes
vstinner
vstinner reviewed Aug 29, 2020
View reviewed changes
@koubaa@vstinner
Update Modules/_sha3/sha3module.c
054ebee
Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner
vstinner approved these changes Sep 1, 2020
View reviewed changes
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@vstinnervstinner merged commit 93d50a6 into python:masterSep 2, 2020
xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
@koubaa
bpo-1635741: Port _sha3 module to multi-phase init (pythonGH-21855)
efdb3a7
Port the _sha3 extension module to multi-phase init (PEP 489). Convert static types to heap types.
@tiran
Copy link
Member

tiran commented Apr 17, 2021

Please respect the code ownership / expert index and do not merge any PRs without approval of a domain expert. This PR introduced a possible security issue.

tiran added a commit to tiran/cpython that referenced this pull request Apr 17, 2021
@tiran@alexhenrie
bpo-43362: Fix invalid free and return check in _sha3 module
6520e49
Commit 93d50a6 / pythonGH-21855 changed the order of variable definitions, which introduced a potential invalid free bug. Py_buffer object is now initialized earlier and the result of Keccak initialize is verified. Co-authored-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Christian Heimes <christian@python.org>
@tirantiran mentioned this pull request Apr 17, 2021
Merged
tiran added a commit that referenced this pull request Apr 18, 2021
@tiran@alexhenrie
bpo-43362: Fix invalid free and return check in _sha3 module (GH-25463)
aa6da32
Commit 93d50a6 / GH-21855 changed the order of variable definitions, which introduced a potential invalid free bug. Py_buffer object is now initialized earlier and the result of Keccak initialize is verified. Co-authored-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Christian Heimes <christian@python.org> Co-authored-by: Alex Henrie <alexhenrie24@gmail.com>
@kylotankylotanmannequin mentioned this pull request Sep 19, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinnervstinnervstinner approved these changes

@shihai1991shihai1991shihai1991 approved these changes

@tirantiranAwaiting requested review from tiran

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@koubaa@tiran@vstinner@shihai1991@the-knights-who-say-ni@bedevere-bot