From 28b18c43fefe6236f899eba79f9a1b2bad134d0f Mon Sep 17 00:00:00 2001 From: Damien George Date: Wed, 31 Jan 2024 12:56:29 +1100 Subject: [PATCH] py/compile: Fix potential Py-stack overflow in try-finally with return. If a return is executed within the try block of a try-finally then the return value is stored on the top of the Python stack during the execution of the finally block. In this case the Python stack is one larger than it normally would be in the finally block. Prior to this commit, the compiler was not taking this case into account and could have a Python stack overflow if the Python stack used by the finally block was more than that used elsewhere in the function. In such a scenario the last argument of the function would be clobbered by the top-most temporary value used in the deepest Python expression/statement. This commit fixes that case by making sure enough Python stack is allocated to the function. Fixes issue #13562. Signed-off-by: Damien George --- py/compile.c | 11 +++++++++++ tests/basics/try_finally_return.py | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/py/compile.c b/py/compile.c index 4f91ca49b9..a9b34ce5d9 100644 --- a/py/compile.c +++ b/py/compile.c @@ -1650,9 +1650,11 @@ STATIC void compile_try_except(compiler_t *comp, mp_parse_node_t pn_body, int n_ if (qstr_exception_local != 0) { EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE); EMIT_ARG(label_assign, l3); + EMIT_ARG(adjust_stack_size, 1); // stack adjust for possible return value EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE); compile_store_id(comp, qstr_exception_local); compile_delete_id(comp, qstr_exception_local); + EMIT_ARG(adjust_stack_size, -1); compile_decrease_except_level(comp); } @@ -1682,9 +1684,18 @@ STATIC void compile_try_finally(compiler_t *comp, mp_parse_node_t pn_body, int n } else { compile_try_except(comp, pn_body, n_except, pn_except, pn_else); } + + // If the code reaches this point then the try part of the try-finally exited normally. + // This is indicated to the runtime by None sitting on the stack. EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE); + + // Compile the finally block. + // The stack needs to be adjusted by 1 to account for the possibility that the finally is + // being executed as part of a return, and the return value is on the top of the stack. EMIT_ARG(label_assign, l_finally_block); + EMIT_ARG(adjust_stack_size, 1); compile_node(comp, pn_finally); + EMIT_ARG(adjust_stack_size, -1); compile_decrease_except_level(comp); } diff --git a/tests/basics/try_finally_return.py b/tests/basics/try_finally_return.py index 31a507e8d0..21e01ea21b 100644 --- a/tests/basics/try_finally_return.py +++ b/tests/basics/try_finally_return.py @@ -70,3 +70,13 @@ def f(): finally: print('finally 1') print(f()) + +# the finally block uses a lot of Python stack and then a local is accessed +# (tests that the use of the stack doesn't clobber the local) +def f(x): + try: + return x + finally: + print(2, 3, 4, 5, 6) + print(x) +print(f(1))