8000 gh-104432: Use `memcpy()` to avoid misaligned loads by chrstphrchvz · Pull Request #104433 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-104432: Use memcpy() to avoid misaligned loads #104433

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

Merged
merged 4 commits into from
Jul 27, 2023
Merged

gh-104432: Use memcpy() to avoid misaligned loads #104433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
382eb9d
gh-104432: Use memcpy() to avoid misaligned loads
chrstphrchvz May 12, 2023
f59d0be
Rename variables as suggested
chrstphrchvz May 13, 2023
6d78a92
Merge branch 'main' into patch-104432
chrstphrchvz Jul 23, 2023
908fa7a
NEWS entry.
gpshead Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2023-07-27-11-47-29.gh-issue-104432.oGHF-z.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix potential unaligned memory access on C APIs involving returned sequences
of `char *` pointers within the :mod:`grp` and :mod:`socket` modules. These
were revealed using a ``-fsaniziter=alignment`` build on ARM macOS. Patch by
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify, I observed this on macOS for Intel. I have not confirmed if this also occurs on macOS for ARM, but I would not be surprised if it still does.

Copy link
Member

Choose a reason for hiding this comment

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

Oops I shouldn't have bothered mentioning the architecture. no big deal though (feel free to make an edit PR if it bothers you).

I chose the word "revealed" in the wording as a way to indicate how it was found which is more interesting. I suspect the alignment sanitizer may flag this regardless of compilation target architecture but I didn't try to reproduce it myself. :)

I expect the fixup applies all over the place. Regardless of native transparent hardware support (x86) or triggering a trap to the OS to fix the unaligned access up (Alpha & a pile of very RISC thing) they are usually less efficient - so directly using code that detects it and does it right rather than triggering an even slower path is good.

Christopher Chavez.
10 changes: 8 additions & 2 deletions Modules/grpmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,14 @@ mkgrent(PyObject *module, struct group *p)
Py_DECREF(v);
return NULL;
}
for (member = p->gr_mem; *member != NULL; member++) {
PyObject *x = PyUnicode_DecodeFSDefault(*member);
for (member = p->gr_mem; ; member++) {
char *group_member;
// member can be misaligned
memcpy(&group_member, member, sizeof(group_member));
if (group_member == NULL) {
break;
}
PyObject *x = PyUnicode_DecodeFSDefault(group_member);
if (x == NULL || PyList_Append(w, x) != 0) {
Py_XDECREF(x);
Py_DECREF(w);
Expand Down
22 changes: 17 additions & 5 deletions Modules/socketmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5779,9 +5779,15 @@ gethost_common(socket_state *state, struct hostent *h, struct sockaddr *addr,

/* SF #1511317: h_aliases can be NULL */
if (h->h_aliases) {
for (pch = h->h_aliases; *pch != NULL; pch++) {
for (pch = h->h_aliases; ; pch++) {
int status;
tmp = PyUnicode_FromString(*pch);
char *host_alias;
// pch can be misaligned
memcpy(&host_alias, pch, sizeof(host_alias));
if (host_alias == NULL) {
break;
}
tmp = PyUnicode_FromString(host_alias);
if (tmp == NULL)
goto err;

Expand All @@ -5793,8 +5799,14 @@ gethost_common(socket_state *state, struct hostent *h, struct sockaddr *addr,
}
}

for (pch = h->h_addr_list; *pch != NULL; pch++) {
for (pch = h->h_addr_list; ; pch++) {
int status;
char *host_address;
// pch can be misaligned
memcpy(&host_address, pch, sizeof(host_address));
if (host_address == NULL) {
break;
}

switch (af) {

Expand All @@ -5806,7 +5818,7 @@ gethost_common(socket_state *state, struct hostent *h, struct sockaddr *addr,
#ifdef HAVE_SOCKADDR_SA_LEN
sin.sin_len = sizeof(sin);
#endif
memcpy(&sin.sin_addr, *pch, sizeof(sin.sin_addr));
memcpy(&sin.sin_addr, host_address, sizeof(sin.sin_addr));
tmp = make_ipv4_addr(&sin);

if (pch == h->h_addr_list && alen >= sizeof(sin))
Expand All @@ -5823,7 +5835,7 @@ gethost_common(socket_state *state, struct hostent *h, struct sockaddr *addr,
#ifdef HAVE_SOCKADDR_SA_LEN
sin6.sin6_len = sizeof(sin6);
#endif
memcpy(&sin6.sin6_addr, *pch, sizeof(sin6.sin6_addr));
memcpy(&sin6.sin6_addr, host_address, sizeof(sin6.sin6_addr));
tmp = make_ipv6_addr(&sin6);

if (pch == h->h_addr_list && alen >= sizeof(sin6))
Expand Down
0