Skip to content

gh-151297: Fix undefined behavior in _PyObject_MiRealloc#151358

Merged
ZeroIntensity merged 8 commits into
python:mainfrom
godlygeek:fix_pyobject_realloc_ub
Jun 12, 2026
Merged

gh-151297: Fix undefined behavior in _PyObject_MiRealloc#151358
ZeroIntensity merged 8 commits into
python:mainfrom
godlygeek:fix_pyobject_realloc_ub

Conversation

@godlygeek

@godlygeek godlygeek commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

The standard says that a call to memcpy must pass a valid source and destination pointer even if the size is 0, so we must avoid calling memcpy when our source pointer is NULL. If we don't, an optimizing compiler can decide that the pointer must be non-NULL based on the presence of UB, and optimize out checks for null pointers.

Specifically, note that the standard says:

Where an argument declared as size_t n specifies the length of the
array for a function, n can have the value zero on a call to that
function. Unless explicitly stated otherwise in the description of
a particular function in this subclause, pointer arguments on such
a call shall still have valid values, as described in 7.1.4.

And section 7.1.4 says:

If an argument to a function has an invalid value (such as a value
outside the domain of the function, or a pointer outside the address
space of the program, or a null pointer, or a pointer to
non-modifiable storage when the corresponding parameter is not
const-qualified) or a type (after default argument promotion) not
expected by a function with a variable number of arguments, the
behavior is undefined.

The specification for memcpy doesn't state that it's allowed to be called with null pointers, and Linux's /usr/include/string.h declares memcpy as __nonnull ((1, 2)).

The standard says that a call to `memcpy` must pass a valid source and
destination pointer even if the size is 0, so we must avoid calling
`memcpy` when our source pointer is NULL. If we don't, an optimizing
compiler can decide that the pointer must be non-NULL based on the
presence of UB, and optimize out checks for null pointers.

Specifically, note that the standard says:

    Where an argument declared as size_t n specifies the length of the
    array for a function, n can have the value zero on a call to that
    function. Unless explicitly stated otherwise in the description of
    a particular function in this subclause, pointer arguments on such
    a call shall still have valid values, as described in 7.1.4.

And section 7.1.4 says:

    If an argument to a function has an invalid value (such as a value
    outside the domain of the function, or a pointer outside the address
    space of the program, or a null pointer, or a pointer to
    non-modifiable storage when the corresponding parameter is not
    const-qualified) or a type (after default argument promotion) not
    expected by a function with a variable number of arguments, the
    behavior is undefined.

The specification for `memcpy` doesn't state that it's allowed to be
called with null pointers, and Linux's `/usr/include/string.h` declares
`memcpy` as `__nonnull ((1, 2))`.

Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just some standard housekeeping nitpicks. The crux of the fix looks good.

I suspect we don't have a test case stressing this path. Would you mind adding one?

Comment thread Misc/NEWS.d/next/Security/2026-06-11-16-03-23.gh-issue-151297.NGPkUM.rst Outdated
Comment thread Objects/obmalloc.c
@ZeroIntensity ZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jun 11, 2026
@godlygeek

Copy link
Copy Markdown
Contributor Author

I suspect we don't have a test case stressing this path. Would you mind adding one?

What would you like the test to look like? It's not really possible to test for a lack of UB...

Or do you just mean a test for realloc(NULL, size) ?

@ZeroIntensity

Copy link
Copy Markdown
Member

Or do you just mean a test for realloc(NULL, size) ?

Yeah, that.

@godlygeek

Copy link
Copy Markdown
Contributor Author

K, I've added one

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@ZeroIntensity ZeroIntensity merged commit c375992 into python:main Jun 12, 2026
58 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @godlygeek for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Sorry, @godlygeek and @ZeroIntensity, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker c37599200f688538efa34a49f262a9a4a899a953 3.14

@bedevere-app

bedevere-app Bot commented Jun 12, 2026

Copy link
Copy Markdown

GH-151388 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 12, 2026
@miss-islington-app

Copy link
Copy Markdown

Sorry, @godlygeek and @ZeroIntensity, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker c37599200f688538efa34a49f262a9a4a899a953 3.13

@ZeroIntensity

Copy link
Copy Markdown
Member

Ah, looks like this doesn't apply to 3.13 and 3.14; the UB was added in 98e55d7, which wasn't backported.

@ZeroIntensity ZeroIntensity removed the needs backport to 3.13 bugs and security fixes label Jun 12, 2026
@ZeroIntensity ZeroIntensity removed the needs backport to 3.14 bugs and security fixes label Jun 12, 2026
ZeroIntensity pushed a commit that referenced this pull request Jun 12, 2026
…-151358) (GH-151388)

The standard says that a call to `memcpy` must pass a valid source and
destination pointer even if the size is 0, so we must avoid calling
`memcpy` when our source pointer is NULL. If we don't, an optimizing
compiler can decide that the pointer must be non-NULL based on the
presence of UB, and optimize out checks for null pointers.

Specifically, note that the standard says:

    Where an argument declared as size_t n specifies the length of the
    array for a function, n can have the value zero on a call to that
    function. Unless explicitly stated otherwise in the description of
    a particular function in this subclause, pointer arguments on such
    a call shall still have valid values, as described in 7.1.4.

And section 7.1.4 says:

    If an argument to a function has an invalid value (such as a value
    outside the domain of the function, or a pointer outside the address
    space of the program, or a null pointer, or a pointer to
    non-modifiable storage when the corresponding parameter is not
    const-qualified) or a type (after default argument promotion) not
    expected by a function with a variable number of arguments, the
    behavior is undefined.

The specification for `memcpy` doesn't state that it's allowed to be
called with null pointers, and Linux's `/usr/include/string.h` declares
`memcpy` as `__nonnull ((1, 2))`.
(cherry picked from commit c375992)

Co-authored-by: Matt Wozniski <mwozniski@bloomberg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants