Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-146308: Fix error handling issues in _remote_debugging module#146309

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
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
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
Fixed multiple error handling issues in the :mod:`!_remote_debugging` module
including a double-free in code object caching, memory leaks on allocation
failure, missing exception checks in binary format varint decoding, reference
leaks on error paths in frame chain processing, and inconsistent thread status
error reporting across platforms. Patch by Pablo Galindo.
1 change: 1 addition & 0 deletionsModules/_remote_debugging/_remote_debugging.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -307,6 +307,7 @@ typedef struct {
#endif
#ifdef __APPLE__
uint64_t thread_id_offset;
int thread_id_offset_initialized;
#endif
#ifdef MS_WINDOWS
PVOID win_process_buffer;
Expand Down
3 changes: 3 additions & 0 deletionsModules/_remote_debugging/asyncio.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -940,6 +940,9 @@ process_running_task_chain(
PyObject *coro_chain = PyStructSequence_GET_ITEM(task_info, 2);
assert(coro_chain != NULL);
if (PyList_GET_SIZE(coro_chain) != 1) {
PyErr_Format(PyExc_RuntimeError,
"Expected single-item coro chain, got %zd items",
PyList_GET_SIZE(coro_chain));
set_exception_cause(unwinder, PyExc_RuntimeError, "Coro chain is not a single item");
return -1;
}
Expand Down
9 changes: 5 additions & 4 deletionsModules/_remote_debugging/binary_io.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -415,8 +415,8 @@ decode_varint_u32(const uint8_t *data, size_t *offset, size_t max_size)
{
size_t saved_offset = *offset;
uint64_t value = decode_varint_u64(data, offset, max_size);
if (PyErr_Occurred()) {
return 0;
if (*offset == saved_offset) {
return 0; /* decode_varint_u64 already set PyErr */
}
if (UNLIKELY(value > UINT32_MAX)) {
*offset = saved_offset;
Expand All@@ -430,9 +430,10 @@ decode_varint_u32(const uint8_t *data, size_t *offset, size_t max_size)
static inline int32_t
decode_varint_i32(const uint8_t *data, size_t *offset, size_t max_size)
{
size_t saved_offset = *offset;
uint32_t zigzag = decode_varint_u32(data, offset, max_size);
if (PyErr_Occurred()) {
return 0;
if (*offset == saved_offset) {
return 0; /* decode_varint_u32 already set PyErr */
}
return (int32_t)((zigzag >> 1) ^ -(int32_t)(zigzag & 1));
}
Expand Down
50 changes: 45 additions & 5 deletionsModules/_remote_debugging/binary_io_reader.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -571,15 +571,16 @@ reader_get_or_create_thread_state(BinaryReader *reader, uint64_t thread_id,
return NULL;
}
} else if (reader->thread_state_count >= reader->thread_state_capacity) {
reader->thread_states = grow_array(reader->thread_states,
&reader->thread_state_capacity,
sizeof(ReaderThreadState));
if (!reader->thread_states) {
ReaderThreadState *new_states = grow_array(reader->thread_states,
&reader->thread_state_capacity,
sizeof(ReaderThreadState));
if (!new_states) {
return NULL;
}
reader->thread_states = new_states;
}

ReaderThreadState *ts = &reader->thread_states[reader->thread_state_count++];
ReaderThreadState *ts = &reader->thread_states[reader->thread_state_count];
memset(ts, 0, sizeof(ReaderThreadState));
ts->thread_id = thread_id;
ts->interpreter_id = interpreter_id;
Expand All@@ -590,6 +591,9 @@ reader_get_or_create_thread_state(BinaryReader *reader, uint64_t thread_id,
PyErr_NoMemory();
return NULL;
}
// Increment count only after successful allocation to avoid
// leaving a half-initialized entry visible to future lookups
reader->thread_state_count++;
return ts;
}

Expand All@@ -604,7 +608,11 @@ static inline int
decode_stack_full(ReaderThreadState *ts, const uint8_t *data,
size_t *offset, size_t max_size)
{
size_t prev_offset = *offset;
uint32_t depth = decode_varint_u32(data, offset, max_size);
if (*offset == prev_offset) {
return -1;
}

/* Validate depth against capacity to prevent buffer overflow */
if (depth > ts->current_stack_capacity) {
Expand All@@ -615,7 +623,11 @@ decode_stack_full(ReaderThreadState *ts, const uint8_t *data,

ts->current_stack_depth = depth;
for (uint32_t i = 0; i < depth; i++) {
size_t prev_offset = *offset;
ts->current_stack[i] = decode_varint_u32(data, offset, max_size);
if (*offset == prev_offset) {
return -1;
}
}
return 0;
}
Expand All@@ -627,8 +639,16 @@ static inline int
decode_stack_suffix(ReaderThreadState *ts, const uint8_t *data,
size_t *offset, size_t max_size)
{
size_t prev_offset = *offset;
uint32_t shared = decode_varint_u32(data, offset, max_size);
if (*offset == prev_offset) {
return -1;
}
prev_offset = *offset;
uint32_t new_count = decode_varint_u32(data, offset, max_size);
if (*offset == prev_offset) {
return -1;
}

/* Validate shared doesn't exceed current stack depth */
if (shared > ts->current_stack_depth) {
Expand DownExpand Up@@ -664,7 +684,11 @@ decode_stack_suffix(ReaderThreadState *ts, const uint8_t *data,
}

for (uint32_t i = 0; i < new_count; i++) {
size_t prev_offset = *offset;
ts->current_stack[i] = decode_varint_u32(data, offset, max_size);
if (*offset == prev_offset) {
return -1;
}
}
ts->current_stack_depth = final_depth;
return 0;
Expand All@@ -677,8 +701,16 @@ static inline int
decode_stack_pop_push(ReaderThreadState *ts, const uint8_t *data,
size_t *offset, size_t max_size)
{
size_t prev_offset = *offset;
uint32_t pop = decode_varint_u32(data, offset, max_size);
if (*offset == prev_offset) {
return -1;
}
prev_offset = *offset;
uint32_t push = decode_varint_u32(data, offset, max_size);
if (*offset == prev_offset) {
return -1;
}
size_t keep = (ts->current_stack_depth > pop) ? ts->current_stack_depth - pop : 0;

/* Validate final depth doesn't exceed capacity */
Expand All@@ -699,7 +731,12 @@ decode_stack_pop_push(ReaderThreadState *ts, const uint8_t *data,
}

for (uint32_t i = 0; i < push; i++) {
size_t prev_offset = *offset;
ts->current_stack[i] = decode_varint_u32(data, offset, max_size);
/* If offset didn't advance, varint decoding failed */
if (*offset == prev_offset) {
return -1;
}
}
ts->current_stack_depth = final_depth;
return 0;
Expand DownExpand Up@@ -1222,6 +1259,9 @@ binary_reader_close(BinaryReader *reader)
reader->mapped_data = NULL; /* Prevent use-after-free */
reader->mapped_size = 0;
}
/* Clear sample_data which may point into the now-unmapped region */
reader->sample_data = NULL;
reader->sample_data_size = 0;
if (reader->fd >= 0) {
close(reader->fd);
reader->fd = -1; /* Mark as closed */
Expand Down
8 changes: 8 additions & 0 deletionsModules/_remote_debugging/code_objects.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,7 @@ cache_tlbc_array(RemoteUnwinderObject *unwinder, uintptr_t code_addr, uintptr_t
void *key = (void *)code_addr;
if (_Py_hashtable_set(unwinder->tlbc_cache, key, entry) < 0) {
tlbc_cache_entry_destroy(entry);
PyErr_NoMemory();
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to store TLBC entry in cache");
return 0; // Cache error
}
Expand DownExpand Up@@ -408,7 +409,14 @@ parse_code_object(RemoteUnwinderObject *unwinder,
meta->addr_code_adaptive = real_address + (uintptr_t)unwinder->debug_offsets.code_object.co_code_adaptive;

if (unwinder && unwinder->code_object_cache && _Py_hashtable_set(unwinder->code_object_cache, key, meta) < 0) {
// Ownership of func/file/linetable was transferred to meta,
// so NULL them before destroying meta to prevent double-free
// in the error label's Py_XDECREF calls.
func = NULL;
file = NULL;
linetable = NULL;
cached_code_metadata_destroy(meta);
PyErr_NoMemory();
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to cache code metadata");
goto error;
}
Expand Down
2 changes: 2 additions & 0 deletionsModules/_remote_debugging/frames.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -348,10 +348,12 @@ process_frame_chain(
PyObject *extra_frame_info = make_frame_info(
unwinder, _Py_LATIN1_CHR('~'), Py_None, extra_frame, Py_None);
if (extra_frame_info == NULL) {
Py_XDECREF(frame);
return -1;
}
if (PyList_Append(ctx->frame_info, extra_frame_info) < 0) {
Py_DECREF(extra_frame_info);
Py_XDECREF(frame);
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to append extra frame");
return -1;
}
Expand Down
1 change: 1 addition & 0 deletionsModules/_remote_debugging/module.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -420,6 +420,7 @@ _remote_debugging_RemoteUnwinder___init___impl(RemoteUnwinderObject *self,

#if defined(__APPLE__)
self->thread_id_offset = 0;
self->thread_id_offset_initialized = 0;
#endif

#ifdef MS_WINDOWS
Expand Down
2 changes: 2 additions & 0 deletionsModules/_remote_debugging/object_reading.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -196,6 +196,8 @@ read_py_long(

// Validate size: reject garbage (negative or unreasonably large)
if (size < 0 || size > MAX_LONG_DIGITS) {
PyErr_Format(PyExc_RuntimeError,
"Invalid PyLong digit count: %zd (expected 0-%d)", size, MAX_LONG_DIGITS);
set_exception_cause(unwinder, PyExc_RuntimeError,
"Invalid PyLong size (corrupted remote memory)");
return -1;
Expand Down
23 changes: 13 additions & 10 deletionsModules/_remote_debugging/threads.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -157,11 +157,11 @@ find_running_frame(
int
get_thread_status(RemoteUnwinderObject *unwinder, uint64_t tid, uint64_t pthread_id) {
#if defined(__APPLE__) && TARGET_OS_OSX
if (unwinder->thread_id_offset == 0) {
if (!unwinder->thread_id_offset_initialized) {
uint64_t *tids = (uint64_t *)PyMem_Malloc(MAX_NATIVE_THREADS * sizeof(uint64_t));
if (!tids) {
PyErr_NoMemory();
return-1;
// Non-fatal: thread status is best-effort
returnTHREAD_STATE_UNKNOWN;
}
int n = proc_pidinfo(unwinder->handle.pid, PROC_PIDLISTTHREADS, 0, tids, MAX_NATIVE_THREADS * sizeof(uint64_t)) / sizeof(uint64_t);
if (n <= 0) {
Expand All@@ -176,6 +176,7 @@ get_thread_status(RemoteUnwinderObject *unwinder, uint64_t tid, uint64_t pthread
}
}
unwinder->thread_id_offset = min_offset;
unwinder->thread_id_offset_initialized = 1;
PyMem_Free(tids);
}
struct proc_threadinfo ti;
Expand DownExpand Up@@ -239,20 +240,21 @@ get_thread_status(RemoteUnwinderObject *unwinder, uint64_t tid, uint64_t pthread
unwinder->win_process_buffer_size = n;
PVOID new_buffer = PyMem_Realloc(unwinder->win_process_buffer, n);
if (!new_buffer) {
return -1;
// Match Linux/macOS: degrade gracefully on alloc failure
return THREAD_STATE_UNKNOWN;
}
unwinder->win_process_buffer = new_buffer;
return get_thread_status(unwinder, tid, pthread_id);
}
if (status != STATUS_SUCCESS) {
return-1;
returnTHREAD_STATE_UNKNOWN;
}

SYSTEM_PROCESS_INFORMATION *pi = (SYSTEM_PROCESS_INFORMATION *)unwinder->win_process_buffer;
while ((ULONG)(ULONG_PTR)pi->UniqueProcessId != unwinder->handle.pid) {
if (pi->NextEntryOffset == 0) {
//We didn't find the process
return-1;
//Process not found (may have exited)
returnTHREAD_STATE_UNKNOWN;
}
pi = (SYSTEM_PROCESS_INFORMATION *)(((BYTE *)pi) + pi->NextEntryOffset);
}
Expand All@@ -264,7 +266,8 @@ get_thread_status(RemoteUnwinderObject *unwinder, uint64_t tid, uint64_t pthread
}
}

return -1;
// Thread not found (may have exited)
return THREAD_STATE_UNKNOWN;
#else
return THREAD_STATE_UNKNOWN;
#endif
Expand DownExpand Up@@ -384,12 +387,12 @@ unwind_stack_for_thread(
long pthread_id = GET_MEMBER(long, ts, unwinder->debug_offsets.thread_state.thread_id);

// Optimization: only check CPU status if needed by mode because it's expensive
int cpu_status =-1;
int cpu_status =THREAD_STATE_UNKNOWN;
if (unwinder->mode == PROFILING_MODE_CPU || unwinder->mode == PROFILING_MODE_ALL) {
cpu_status = get_thread_status(unwinder, tid, pthread_id);
}

if (cpu_status ==-1) {
if (cpu_status ==THREAD_STATE_UNKNOWN) {
status_flags |= THREAD_STATUS_UNKNOWN;
} else if (cpu_status == THREAD_STATE_RUNNING) {
status_flags |= THREAD_STATUS_ON_CPU;
Expand Down
Loading
[8]ページ先頭
©2009-2026 Movatter.jp