Skip to content

Commit 41e9404

Browse files
committed
Only raise an exception (or fatal error) when the stack pointer is about to overflow the stack.
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 4867f71 commit 41e9404

File tree

4 files changed

+34
-9
lines changed

4 files changed

+34
-9
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 becuase 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 limts 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 are 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: 17 additions & 6 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,16 @@ 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+
#if _Py_STACK_GROWS_DOWN
498+
uintptr_t top_addr = _Py_SIZE_ROUND_UP(sp + 8*sizeof(void*), 4096);
496499
*top = top_addr;
497500
*base = top_addr - Py_C_STACK_SIZE;
501+
# else
502+
uintptr_t base_addr = _Py_SIZE_ROUND_DOWN(sp - 8*sizeof(void*), 4096);
503+
*base = base_addr;
504+
*top = base_addr + Py_C_STACK_SIZE;
505+
#endif
498506
#endif
499507
}
500508

@@ -543,7 +551,8 @@ void
543551
_Py_InitializeRecursionLimits(PyThreadState *tstate)
544552
{
545553
uintptr_t base, top;
546-
hardware_stack_limits(&base, &top);
554+
uintptr_t here_addr = _Py_get_machine_stack_pointer();
555+
hardware_stack_limits(&base, &top, here_addr);
547556
assert(top != 0);
548557

549558
tstate_set_stack(tstate, base, top);
@@ -596,10 +605,12 @@ _Py_CheckRecursiveCall(PyThreadState *tstate, const char *where)
596605
assert(_tstate->c_stack_soft_limit != 0);
597606
assert(_tstate->c_stack_hard_limit != 0);
598607
#if _Py_STACK_GROWS_DOWN
608+
assert(here_addr >= _tstate->c_stack_hard_limit - _PyOS_STACK_MARGIN_BYTES);
599609
if (here_addr < _tstate->c_stack_hard_limit) {
600610
/* Overflowing while handling an overflow. Give up. */
601611
int kbytes_used = (int)(_tstate->c_stack_top - here_addr)/1024;
602612
#else
613+
assert(here_addr <= _tstate->c_stack_hard_limit + _PyOS_STACK_MARGIN_BYTES);
603614
if (here_addr > _tstate->c_stack_hard_limit) {
604615
/* Overflowing while handling an overflow. Give up. */
605616
int kbytes_used = (int)(here_addr - _tstate->c_stack_top)/1024;

0 commit comments

Comments
 (0)