From c11cf6c0678fd2f74bb6abeeff101c5d97cc41f2 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 6 Sep 2022 17:10:12 +0100 Subject: [PATCH 1/4] Avoid undefined behavior. --- Include/internal/pycore_frame.h | 24 ++++++++++++++++-------- Python/pystate.c | 14 +++++--------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index efdcdbcf8df805..09f80a9e9345fb 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -192,17 +192,25 @@ _PyFrame_LocalsToFast(_PyInterpreterFrame *frame, int clear); extern _PyInterpreterFrame * _PyThreadState_BumpFramePointerSlow(PyThreadState *tstate, size_t size); +static inline bool +_PyThreadState_HasStackSpace(PyThreadState *tstate, int size) +{ + assert( + (tstate->datastack_top == NULL && tstate->datastack_limit == NULL) + || + (tstate->datastack_top != NULL && tstate->datastack_limit != NULL) + ); + return tstate->datastack_top != NULL && + size < tstate->datastack_limit - tstate->datastack_top; +} + static inline _PyInterpreterFrame * _PyThreadState_BumpFramePointer(PyThreadState *tstate, size_t size) { - PyObject **base = tstate->datastack_top; - if (base) { - PyObject **top = base + size; - assert(tstate->datastack_limit); - if (top < tstate->datastack_limit) { - tstate->datastack_top = top; - return (_PyInterpreterFrame *)base; - } + if (_PyThreadState_HasStackSpace(tstate, size)) { + _PyInterpreterFrame *res = (_PyInterpreterFrame *)tstate->datastack_top; + tstate->datastack_top += size; + return res; } return _PyThreadState_BumpFramePointerSlow(tstate, size); } diff --git a/Python/pystate.c b/Python/pystate.c index a0c12bac9d1408..1cc997d65f3227 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2178,16 +2178,12 @@ push_chunk(PyThreadState *tstate, int size) _PyInterpreterFrame * _PyThreadState_BumpFramePointerSlow(PyThreadState *tstate, size_t size) { - assert(size < INT_MAX/sizeof(PyObject *)); - PyObject **base = tstate->datastack_top; - PyObject **top = base + size; - if (top >= tstate->datastack_limit) { - base = push_chunk(tstate, (int)size); + if (_PyThreadState_HasStackSpace(tstate, size)) { + _PyInterpreterFrame *res = (_PyInterpreterFrame *)tstate->datastack_top; + tstate->datastack_top += size; + return res; } - else { - tstate->datastack_top = top; - } - return (_PyInterpreterFrame *)base; + return (_PyInterpreterFrame *)push_chunk(tstate, (int)size); } void From 4042eace709208b04e9553e472d9bae65838da16 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 5 Sep 2022 17:15:38 +0100 Subject: [PATCH 2/4] Add news. --- .../2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst b/Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst new file mode 100644 index 00000000000000..4734d3d6ded12a --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst @@ -0,0 +1 @@ +Remove two cases of undefined behavoir, by adding NULL checks. From dff7afb811d0259b5267823f22814143b0a0081b Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 7 Sep 2022 10:19:17 +0100 Subject: [PATCH 3/4] Add cast and bounds check. --- Include/internal/pycore_frame.h | 4 ++-- Python/pystate.c | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 09f80a9e9345fb..ecc8346f5ef0b5 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -193,7 +193,7 @@ extern _PyInterpreterFrame * _PyThreadState_BumpFramePointerSlow(PyThreadState *tstate, size_t size); static inline bool -_PyThreadState_HasStackSpace(PyThreadState *tstate, int size) +_PyThreadState_HasStackSpace(PyThreadState *tstate, size_t size) { assert( (tstate->datastack_top == NULL && tstate->datastack_limit == NULL) @@ -201,7 +201,7 @@ _PyThreadState_HasStackSpace(PyThreadState *tstate, int size) (tstate->datastack_top != NULL && tstate->datastack_limit != NULL) ); return tstate->datastack_top != NULL && - size < tstate->datastack_limit - tstate->datastack_top; + size < (size_t)(tstate->datastack_limit - tstate->datastack_top); } static inline _PyInterpreterFrame * diff --git a/Python/pystate.c b/Python/pystate.c index 1cc997d65f3227..3a8140badfb531 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2183,6 +2183,10 @@ _PyThreadState_BumpFramePointerSlow(PyThreadState *tstate, size_t size) tstate->datastack_top += size; return res; } + if (size > INT_MAX/2) { + PyErr_NoMemory(); + return NULL; + } return (_PyInterpreterFrame *)push_chunk(tstate, (int)size); } From bea52cdc867088ee610dd11e059985b1125daf25 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 7 Sep 2022 15:58:56 +0100 Subject: [PATCH 4/4] Update Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst Co-authored-by: Michael Droettboom --- .../2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst b/Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst index 4734d3d6ded12a..1dc4c082bce066 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-09-05-16-43-44.gh-issue-96569.9lmTCC.rst @@ -1 +1 @@ -Remove two cases of undefined behavoir, by adding NULL checks. +Remove two cases of undefined behavior, by adding NULL checks.