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.

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

bpo-28055: fix unaligned accesses in siphash24() #6123

Merged
merged 1 commit into from May 13, 2018
Merged

bpo-28055: fix unaligned accesses in siphash24() #6123

merged 1 commit into from May 13, 2018

Conversation

DerDakon
Copy link
Contributor

@DerDakon DerDakon commented Mar 15, 2018

The hash implementation casts the input pointer to uint64_t* and directly reads from this, which may cause unaligned accesses. Use memcpy() instead so this code will not crash with SIGBUS on sparc.

https://bugs.gentoo.org/show_bug.cgi?id=636400

https://bugs.python.org/issue28055

@the-knights-who-say-ni
Copy link

the-knights-who-say-ni commented Mar 15, 2018

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Please open an issue on the bug tracker for discussion.

@bedevere-bot
Copy link

bedevere-bot commented Mar 26, 2018

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.

@DerDakon
Copy link
Contributor Author

DerDakon commented Mar 26, 2018

@serhiy-storchaka serhiy-storchaka changed the title fix unaligned accesses in siphash24() bpo-28055: fix unaligned accesses in siphash24() Mar 26, 2018
Python/pyhash.c Outdated
v3 ^= mi;
DOUBLE_ROUND(v0,v1,v2,v3);
v0 ^= mi;
}

t = 0;
pt = (uint8_t *)&t;
m = (uint8_t *)in;
m = in;
Copy link
Member

@serhiy-storchaka serhiy-storchaka Mar 26, 2018

Choose a reason for hiding this comment

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

m is not needed. Just use in.

@ned-deily
Copy link
Member

ned-deily commented Apr 6, 2018

So other than a NEWS entry, is there anything else that needs to be done for this before merging?

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Just a news entry is needed.

@DerDakon
Copy link
Contributor Author

DerDakon commented May 6, 2018

Ping

@@ -0,0 +1 @@
fix unaligned accesses in siphash24()
Copy link
Member

@serhiy-storchaka serhiy-storchaka May 13, 2018

Choose a reason for hiding this comment

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

Start the sentence from the title case, add missed period, and please add "Patch by yourname."

Copy link
Contributor Author

@DerDakon DerDakon May 13, 2018

Choose a reason for hiding this comment

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

Fixed.

The hash implementation casts the input pointer to uint64_t* and directly reads
from this, which may cause unaligned accesses. Use memcpy() instead so this code
will not crash with SIGBUS on sparc.

https://bugs.gentoo.org/show_bug.cgi?id=636400
@serhiy-storchaka serhiy-storchaka merged commit 1e2ec8a into python:master May 13, 2018
@miss-islington
Copy link
Contributor

miss-islington commented May 13, 2018

Thanks @DerDakon for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 13, 2018
The hash implementation casts the input pointer to uint64_t* and directly reads
from this, which may cause unaligned accesses. Use memcpy() instead so this code
will not crash with SIGBUS on sparc.

https://bugs.gentoo.org/show_bug.cgi?id=636400
(cherry picked from commit 1e2ec8a)

Co-authored-by: Rolf Eike Beer <eike@sf-mail.de>
@bedevere-bot
Copy link

bedevere-bot commented May 13, 2018

GH-6777 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 13, 2018
The hash implementation casts the input pointer to uint64_t* and directly reads
from this, which may cause unaligned accesses. Use memcpy() instead so this code
will not crash with SIGBUS on sparc.

https://bugs.gentoo.org/show_bug.cgi?id=636400
(cherry picked from commit 1e2ec8a)

Co-authored-by: Rolf Eike Beer <eike@sf-mail.de>
@bedevere-bot
Copy link

bedevere-bot commented May 13, 2018

GH-6778 is a backport of this pull request to the 3.6 branch.

miss-islington added a commit that referenced this pull request May 13, 2018
The hash implementation casts the input pointer to uint64_t* and directly reads
from this, which may cause unaligned accesses. Use memcpy() instead so this code
will not crash with SIGBUS on sparc.

https://bugs.gentoo.org/show_bug.cgi?id=636400
(cherry picked from commit 1e2ec8a)

Co-authored-by: Rolf Eike Beer <eike@sf-mail.de>
miss-islington added a commit that referenced this pull request May 13, 2018
The hash implementation casts the input pointer to uint64_t* and directly reads
from this, which may cause unaligned accesses. Use memcpy() instead so this code
will not crash with SIGBUS on sparc.

https://bugs.gentoo.org/show_bug.cgi?id=636400
(cherry picked from commit 1e2ec8a)

Co-authored-by: Rolf Eike Beer <eike@sf-mail.de>
yahya-abou-imran pushed a commit to yahya-abou-imran/cpython that referenced this pull request Nov 2, 2018
The hash implementation casts the input pointer to uint64_t* and directly reads
from this, which may cause unaligned accesses. Use memcpy() instead so this code
will not crash with SIGBUS on sparc.

https://bugs.gentoo.org/show_bug.cgi?id=636400
matoro added a commit to matoro/cpython that referenced this pull request Jun 22, 2022
Like python#6123 this pointer may be
unaligned, so a memcpy() instead of simple assignment is required for
strict architectures e.g. sparc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants