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

sys_getwindowsversion_impl might potentially swallow errors in sysmodule.c #116326

Closed
sobolevn opened this issue Mar 4, 2024 · 0 comments
Closed
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Mar 4, 2024

Bug report

While merging #115321 I've noticed a similar issue to #115320:

cpython/Python/sysmodule.c

Lines 1661 to 1699 in 207030f

PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwMajorVersion));
PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwMinorVersion));
PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwBuildNumber));
PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.dwPlatformId));
PyStructSequence_SET_ITEM(version, pos++, PyUnicode_FromWideChar(ver.szCSDVersion, -1));
PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wServicePackMajor));
PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wServicePackMinor));
PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wSuiteMask));
PyStructSequence_SET_ITEM(version, pos++, PyLong_FromLong(ver.wProductType));
// GetVersion will lie if we are running in a compatibility mode.
// We need to read the version info from a system file resource
// to accurately identify the OS version. If we fail for any reason,
// just return whatever GetVersion said.
PyObject *realVersion = _sys_getwindowsversion_from_kernel32();
if (!realVersion) {
PyErr_Clear();
realVersion = Py_BuildValue("(kkk)",
ver.dwMajorVersion,
ver.dwMinorVersion,
ver.dwBuildNumber
);
}
if (realVersion) {
PyStructSequence_SET_ITEM(version, pos++, realVersion);
}
if (PyErr_Occurred()) {
Py_DECREF(version);
return NULL;
}
if (PyObject_SetAttrString(module, "_cached_windows_version", version) < 0) {
Py_DECREF(version);
return NULL;
}
return version;

It can also overwrite errors, I will send a PR similar to #115321 to fix this as well.

Linked PRs

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 4, 2024
@sobolevn sobolevn self-assigned this Mar 4, 2024
sobolevn added a commit to sobolevn/cpython that referenced this issue Mar 4, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 5, 2024
…sysmodule` (pythonGH-116339)

(cherry picked from commit c91bdf8)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit that referenced this issue Mar 5, 2024
…`sysmodule` (GH-116339) (#116354)

* gh-116326: Handler errors correctly in `getwindowsversion` in `sysmodule` (GH-116339)
(cherry picked from commit c91bdf8)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit to sobolevn/cpython that referenced this issue Mar 5, 2024
…n` in `sysmodule` (pythonGH-116339)

(cherry picked from commit c91bdf8)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
sobolevn added a commit that referenced this issue Mar 6, 2024
@sobolevn sobolevn closed this as completed Mar 6, 2024
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant