Skip to content

gh-144319: Fix huge page leak in datastack chunk allocator #147963

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

Open
pablogsal wants to merge 1 commit into
base: main
Choose a base branch
from
+26 −4
Open

gh-144319: Fix huge page leak in datastack chunk allocator #147963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2026-04-01-12-52-31.gh-issue-144319.iZk4hs.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix a bug that could cause applications with specific allocation patterns to
leak memory via Huge Pages if compiled with Huge Page support. Patch by
Pablo Galindo
7 changes: 6 additions & 1 deletion Objects/obmalloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <stdlib.h> // malloc()
#include <stdbool.h>
#include <stdio.h> // fopen(), fgets(), sscanf()
#include <errno.h> // errno
#ifdef WITH_MIMALLOC
// Forward declarations of functions used in our mimalloc modifications
static void _PyMem_mi_page_clear_qsbr(mi_page_t *page);
Expand Down Expand Up @@ -648,7 +649,11 @@ _PyMem_ArenaFree(void *Py_UNUSED(ctx), void *ptr,
if (ptr == NULL) {
return;
}
munmap(ptr, size);
if (munmap(ptr, size) < 0) {
_Py_FatalErrorFormat(__func__,
"munmap(%p, %zu) failed with errno %d",
ptr, size, errno);
}
#else
free(ptr);
#endif
Expand Down
20 changes: 17 additions & 3 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1433,16 +1433,30 @@ tstate_is_alive(PyThreadState *tstate)
// lifecycle
//----------

/* When huge pages are enabled, _PyObject_VirtualAlloc may back small requests
* with a full 2MB huge page. We must pass the *mapped* size (not the
* requested size) to _PyObject_VirtualFree, otherwise munmap receives a
* sub-hugepage length and fails with EINVAL, leaking the huge page. */
#if defined(PYMALLOC_USE_HUGEPAGES) && defined(__linux__)
# define HUGEPAGE_SIZE (2U << 20) /* 2 MB */
Copy link
Copy Markdown
Contributor

@maurycy maurycy Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe it could be a helper instead, leveraging

cpython/Objects/obmalloc.c

Line 546 in 4497cf3

_pymalloc_system_hugepage_size(void)
as the page size can vary? Very lazy idea is adding _PyObject_VirtualAllocRounded, or similar

# define ROUND_UP_HUGEPAGE(n) \
(((n) + HUGEPAGE_SIZE - 1) & ~(HUGEPAGE_SIZE - 1))
#else
# define ROUND_UP_HUGEPAGE(n) (n)
#endif


static _PyStackChunk*
allocate_chunk(int size_in_bytes, _PyStackChunk* previous)
{
assert(size_in_bytes % sizeof(PyObject **) == 0);
_PyStackChunk *res = _PyObject_VirtualAlloc(size_in_bytes);
size_t mapped_size = ROUND_UP_HUGEPAGE(size_in_bytes);
_PyStackChunk *res = _PyObject_VirtualAlloc(mapped_size);
Copy link
Copy Markdown
Contributor

@maurycy maurycy Apr 1, 2026

Choose a reason for hiding this comment

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

Out of curiosity, why using PyMem_RawMalloc wouldn't be easier?

The comment even states:

cpython/Python/pystate.c

Line 33 in 4497cf3

Always use PyMem_RawMalloc() and PyMem_RawFree() directly in this file. A

Copy link
Copy Markdown
Member Author

@pablogsal pablogsal Apr 1, 2026

Choose a reason for hiding this comment

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

Yes but I want to preserve the original intent of the code for the minimum bugfix. I haven't think of the subtle consequences of just getting a malloc chunk yet....

Copy link
Copy Markdown
Contributor

@maurycy maurycy Apr 1, 2026

Choose a reason for hiding this comment

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

I see! Does

cpython/Python/optimizer.c

Line 1010 in 4497cf3

_tstate->jit_tracer_state = (_PyJitTracerState *)_PyObject_VirtualAlloc(sizeof(_PyJitTracerState));
need rounding, too?

if (res == NULL) {
return NULL;
}
res->previous = previous;
res->size = size_in_bytes;
res->size = mapped_size;
res->top = 0;
return res;
}
Expand Down Expand Up @@ -3082,7 +3096,7 @@ push_chunk(PyThreadState *tstate, int size)
&tstate->datastack_chunk->data[0];
}
tstate->datastack_chunk = new;
tstate->datastack_limit = (PyObject **)(((char *)new) + allocate_size);
tstate->datastack_limit = (PyObject **)(((char *)new) + new->size);
// When new is the "root" chunk (i.e. new->previous == NULL), we can keep
// _PyThreadState_PopFrame from freeing it later by "skipping" over the
// first element:
Expand Down
Loading