Skip to content

Commit c25a070

Browse files
authored
pythonGH-139653: Only raise an exception (or fatal error) when the stack pointer is about to overflow the stack. (pythonGH-141711)
Only raises if the stack pointer is both below the limit *and* above the stack base. This prevents false positives for user-space threads, as the stack pointer will be outside those bounds if the stack has been swapped.
1 parent 5d1f8f2 commit c25a070

File tree

4 files changed

+38
-10
lines changed

4 files changed

+38
-10
lines changed

Include/internal/pycore_ceval.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,10 +217,13 @@ extern void _PyEval_DeactivateOpCache(void);
217217
static inline int _Py_MakeRecCheck(PyThreadState *tstate) {
218218
uintptr_t here_addr = _Py_get_machine_stack_pointer();
219219
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
220+
// Overflow if stack pointer is between soft limit and the base of the hardware stack.
221+
// If it is below the hardware stack base, assume that we have the wrong stack limits, and do nothing.
222+
// We could have the wrong stack limits because of limited platform support, or user-space threads.
220223
#if _Py_STACK_GROWS_DOWN
221-
return here_addr < _tstate->c_stack_soft_limit;
224+
return here_addr < _tstate->c_stack_soft_limit && here_addr >= _tstate->c_stack_soft_limit - 2 * _PyOS_STACK_MARGIN_BYTES;
222225
#else
223-
return here_addr > _tstate->c_stack_soft_limit;
226+
return here_addr > _tstate->c_stack_soft_limit && here_addr <= _tstate->c_stack_soft_limit + 2 * _PyOS_STACK_MARGIN_BYTES;
224227
#endif
225228
}
226229

InternalDocs/stack_protection.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,19 @@ Recursion checks are performed by `_Py_EnterRecursiveCall()` or `_Py_EnterRecurs
3838

3939
```python
4040
kb_used = (stack_top - stack_pointer)>>10
41-
if stack_pointer < hard_limit:
41+
if stack_pointer < bottom_of_machine_stack:
42+
pass # Our stack limits could be wrong so it is safest to do nothing.
43+
elif stack_pointer < hard_limit:
4244
FatalError(f"Unrecoverable stack overflow (used {kb_used} kB)")
4345
elif stack_pointer < soft_limit:
4446
raise RecursionError(f"Stack overflow (used {kb_used} kB)")
4547
```
4648

49+
### User space threads and other oddities
50+
51+
Some libraries provide user-space threads. These will change the C stack at runtime.
52+
To guard against this we only raise if the stack pointer is in the window between the expected stack base and the soft limit.
53+
4754
### Diagnosing and fixing stack overflows
4855

4956
For stack protection to work correctly the amount of stack consumed between calls to `_Py_EnterRecursiveCall()` must be less than `_PyOS_STACK_MARGIN_BYTES`.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Only raise a ``RecursionError`` or trigger a fatal error if the stack
2+
pointer is both below the limit pointer *and* above the stack base. If
3+
outside of these bounds assume that it is OK. This prevents false positives
4+
when user-space threads swap stacks.

Python/ceval.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -362,9 +362,11 @@ _Py_ReachedRecursionLimitWithMargin(PyThreadState *tstate, int margin_count)
362362
_Py_InitializeRecursionLimits(tstate);
363363
}
364364
#if _Py_STACK_GROWS_DOWN
365-
return here_addr <= _tstate->c_stack_soft_limit + margin_count * _PyOS_STACK_MARGIN_BYTES;
365+
return here_addr <= _tstate->c_stack_soft_limit + margin_count * _PyOS_STACK_MARGIN_BYTES &&
366+
here_addr >= _tstate->c_stack_soft_limit - 2 * _PyOS_STACK_MARGIN_BYTES;
366367
#else
367-
return here_addr > _tstate->c_stack_soft_limit - margin_count * _PyOS_STACK_MARGIN_BYTES;
368+
return here_addr > _tstate->c_stack_soft_limit - margin_count * _PyOS_STACK_MARGIN_BYTES &&
369+
here_addr <= _tstate->c_stack_soft_limit + 2 * _PyOS_STACK_MARGIN_BYTES;
368370
#endif
369371
}
370372

@@ -455,7 +457,7 @@ int pthread_attr_destroy(pthread_attr_t *a)
455457
#endif
456458

457459
static void
458-
hardware_stack_limits(uintptr_t *base, uintptr_t *top)
460+
hardware_stack_limits(uintptr_t *base, uintptr_t *top, uintptr_t sp)
459461
{
460462
#ifdef WIN32
461463
ULONG_PTR low, high;
@@ -491,10 +493,19 @@ hardware_stack_limits(uintptr_t *base, uintptr_t *top)
491493
return;
492494
}
493495
# endif
494-
uintptr_t here_addr = _Py_get_machine_stack_pointer();
495-
uintptr_t top_addr = _Py_SIZE_ROUND_UP(here_addr, 4096);
496+
// Add some space for caller function then round to minimum page size
497+
// This is a guess at the top of the stack, but should be a reasonably
498+
// good guess if called from _PyThreadState_Attach when creating a thread.
499+
// If the thread is attached deep in a call stack, then the guess will be poor.
500+
#if _Py_STACK_GROWS_DOWN
501+
uintptr_t top_addr = _Py_SIZE_ROUND_UP(sp + 8*sizeof(void*), SYSTEM_PAGE_SIZE);
496502
*top = top_addr;
497503
*base = top_addr - Py_C_STACK_SIZE;
504+
# else
505+
uintptr_t base_addr = _Py_SIZE_ROUND_DOWN(sp - 8*sizeof(void*), SYSTEM_PAGE_SIZE);
506+
*base = base_addr;
507+
*top = base_addr + Py_C_STACK_SIZE;
508+
#endif
498509
#endif
499510
}
500511

@@ -543,7 +554,8 @@ void
543554
_Py_InitializeRecursionLimits(PyThreadState *tstate)
544555
{
545556
uintptr_t base, top;
546-
hardware_stack_limits(&base, &top);
557+
uintptr_t here_addr = _Py_get_machine_stack_pointer();
558+
hardware_stack_limits(&base, &top, here_addr);
547559
assert(top != 0);
548560

549561
tstate_set_stack(tstate, base, top);
@@ -587,7 +599,7 @@ PyUnstable_ThreadState_ResetStackProtection(PyThreadState *tstate)
587599

588600

589601
/* The function _Py_EnterRecursiveCallTstate() only calls _Py_CheckRecursiveCall()
590-
if the recursion_depth reaches recursion_limit. */
602+
if the stack pointer is between the stack base and c_stack_hard_limit. */
591603
int
592604
_Py_CheckRecursiveCall(PyThreadState *tstate, const char *where)
593605
{
@@ -596,10 +608,12 @@ _Py_CheckRecursiveCall(PyThreadState *tstate, const char *where)
596608
assert(_tstate->c_stack_soft_limit != 0);
597609
assert(_tstate->c_stack_hard_limit != 0);
598610
#if _Py_STACK_GROWS_DOWN
611+
assert(here_addr >= _tstate->c_stack_hard_limit - _PyOS_STACK_MARGIN_BYTES);
599612
if (here_addr < _tstate->c_stack_hard_limit) {
600613
/* Overflowing while handling an overflow. Give up. */
601614
int kbytes_used = (int)(_tstate->c_stack_top - here_addr)/1024;
602615
#else
616+
assert(here_addr <= _tstate->c_stack_hard_limit + _PyOS_STACK_MARGIN_BYTES);
603617
if (here_addr > _tstate->c_stack_hard_limit) {
604618
/* Overflowing while handling an overflow. Give up. */
605619
int kbytes_used = (int)(here_addr - _tstate->c_stack_top)/1024;

0 commit comments

Comments
 (0)