Mercurial > octave
changeset 28796:7e0712b3388f stable
avoid out-of-bounds element access in call stack (bug #59189)
* call-stack.h, call-stack.cc (call_stack::get_static_link): Delete.
(call_stack::get_new_frame_index_and_links): New function.
(call_stack::push): Use get_new_frame_index_and_links to eliminate
some duplicate code and avoid accessing m_cs[0] when pushing the first
element on the stack. Defer updating m_curr_frame until after the
new element has been pushed on the stack.
author | John W. Eaton <jwe@octave.org> |
---|---|
date | Sun, 27 Sep 2020 12:24:24 -0400 |
parents | c5953e65c6aa |
children | ac5461b59b93 3719f5d452d4 |
files | libinterp/corefcn/call-stack.cc libinterp/corefcn/call-stack.h |
diffstat | 2 files changed, 66 insertions(+), 59 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/call-stack.cc Fri Sep 25 09:43:53 2020 -0400 +++ b/libinterp/corefcn/call-stack.cc Sun Sep 27 12:24:24 2020 -0400 @@ -345,117 +345,122 @@ return retval; } - std::shared_ptr<stack_frame> - call_stack::get_static_link (size_t prev_frame) const + void call_stack::get_new_frame_index_and_links + (size_t& new_frame_idx, std::shared_ptr<stack_frame>& parent_link, + std::shared_ptr<stack_frame>& static_link) const { // FIXME: is there a better way? - std::shared_ptr<stack_frame> slink; + size_t prev_frame_idx = m_curr_frame; + + new_frame_idx = m_cs.size (); - if (m_curr_frame > 0) - { - octave_function *t_fcn = m_cs[prev_frame]->function (); + // m_max_stack_depth should never be less than zero. + if (new_frame_idx > static_cast<size_t> (m_max_stack_depth)) + error ("max_stack_depth exceeded"); + + // There can't be any links to previous frames if this is the first + // frame on the stack. - slink = (t_fcn - ? (t_fcn->is_user_code () - ? m_cs[prev_frame] : m_cs[prev_frame]->static_link ()) - : m_cs[prev_frame]); - } + if (new_frame_idx == 0) + return; + + parent_link = m_cs[prev_frame_idx]; - return slink; + octave_function *t_fcn = parent_link->function (); + + static_link = (t_fcn + ? (t_fcn->is_user_code () + ? parent_link : parent_link->static_link ()) + : parent_link); } void call_stack::push (const symbol_scope& scope) { - size_t prev_frame = m_curr_frame; - m_curr_frame = m_cs.size (); + size_t new_frame_idx; + std::shared_ptr<stack_frame> parent_link; + std::shared_ptr<stack_frame> static_link; - // m_max_stack_depth should never be less than zero. - if (m_curr_frame > static_cast<size_t> (m_max_stack_depth)) - error ("max_stack_depth exceeded"); - - std::shared_ptr<stack_frame> slink = get_static_link (prev_frame); + get_new_frame_index_and_links (new_frame_idx, parent_link, static_link); std::shared_ptr<stack_frame> - new_frame (stack_frame::create (m_evaluator, scope, m_curr_frame, - m_cs[prev_frame], slink)); + new_frame (stack_frame::create (m_evaluator, scope, new_frame_idx, + parent_link, static_link)); m_cs.push_back (new_frame); + + m_curr_frame = new_frame_idx; } void call_stack::push (octave_user_function *fcn, const std::shared_ptr<stack_frame>& closure_frames) { - size_t prev_frame = m_curr_frame; - m_curr_frame = m_cs.size (); + size_t new_frame_idx; + std::shared_ptr<stack_frame> parent_link; + std::shared_ptr<stack_frame> static_link; - // m_max_stack_depth should never be less than zero. - if (m_curr_frame > static_cast<size_t> (m_max_stack_depth)) - error ("max_stack_depth exceeded"); - - std::shared_ptr<stack_frame> slink = get_static_link (prev_frame); + get_new_frame_index_and_links (new_frame_idx, parent_link, static_link); std::shared_ptr<stack_frame> - new_frame (stack_frame::create (m_evaluator, fcn, m_curr_frame, - m_cs[prev_frame], slink, + new_frame (stack_frame::create (m_evaluator, fcn, new_frame_idx, + parent_link, static_link, closure_frames)); m_cs.push_back (new_frame); + + m_curr_frame = new_frame_idx; } void call_stack::push (octave_user_function *fcn, const stack_frame::local_vars_map& local_vars) { - size_t prev_frame = m_curr_frame; - m_curr_frame = m_cs.size (); + size_t new_frame_idx; + std::shared_ptr<stack_frame> parent_link; + std::shared_ptr<stack_frame> static_link; - // m_max_stack_depth should never be less than zero. - if (m_curr_frame > static_cast<size_t> (m_max_stack_depth)) - error ("max_stack_depth exceeded"); - - std::shared_ptr<stack_frame> slink = get_static_link (prev_frame); + get_new_frame_index_and_links (new_frame_idx, parent_link, static_link); std::shared_ptr<stack_frame> - new_frame (stack_frame::create (m_evaluator, fcn, m_curr_frame, - m_cs[prev_frame], slink, local_vars)); + new_frame (stack_frame::create (m_evaluator, fcn, new_frame_idx, + parent_link, static_link, local_vars)); m_cs.push_back (new_frame); + + m_curr_frame = new_frame_idx; } void call_stack::push (octave_user_script *script) { - size_t prev_frame = m_curr_frame; - m_curr_frame = m_cs.size (); + size_t new_frame_idx; + std::shared_ptr<stack_frame> parent_link; + std::shared_ptr<stack_frame> static_link; - // m_max_stack_depth should never be less than zero. - if (m_curr_frame > static_cast<size_t> (m_max_stack_depth)) - error ("max_stack_depth exceeded"); - - std::shared_ptr<stack_frame> slink = get_static_link (prev_frame); + get_new_frame_index_and_links (new_frame_idx, parent_link, static_link); std::shared_ptr<stack_frame> - new_frame (stack_frame::create (m_evaluator, script, m_curr_frame, - m_cs[prev_frame], slink)); + new_frame (stack_frame::create (m_evaluator, script, new_frame_idx, + parent_link, static_link)); m_cs.push_back (new_frame); + + m_curr_frame = new_frame_idx; } void call_stack::push (octave_function *fcn) { - size_t prev_frame = m_curr_frame; - m_curr_frame = m_cs.size (); + size_t new_frame_idx; + std::shared_ptr<stack_frame> parent_link; + std::shared_ptr<stack_frame> static_link; - // m_max_stack_depth should never be less than zero. - if (m_curr_frame > static_cast<size_t> (m_max_stack_depth)) - error ("max_stack_depth exceeded"); - - std::shared_ptr<stack_frame> slink = get_static_link (prev_frame); + get_new_frame_index_and_links (new_frame_idx, parent_link, static_link); std::shared_ptr<stack_frame> - new_frame (stack_frame::create (m_evaluator, fcn, m_curr_frame, - m_cs[prev_frame], slink)); + new_frame (stack_frame::create (m_evaluator, fcn, new_frame_idx, + parent_link, static_link)); m_cs.push_back (new_frame); + + m_curr_frame = new_frame_idx; } bool call_stack::goto_frame (size_t n, bool verbose)
--- a/libinterp/corefcn/call-stack.h Fri Sep 25 09:43:53 2020 -0400 +++ b/libinterp/corefcn/call-stack.h Sun Sep 27 12:24:24 2020 -0400 @@ -153,8 +153,6 @@ // Return TRUE if all elements on the call stack are scripts. bool all_scripts (void) const; - std::shared_ptr<stack_frame> get_static_link (size_t prev_frame) const; - void push (const symbol_scope& scope); void push (octave_user_function *fcn, @@ -306,6 +304,10 @@ private: + void get_new_frame_index_and_links + (size_t& new_frame_idx, std::shared_ptr<stack_frame>& parent_link, + std::shared_ptr<stack_frame>& static_link) const; + tree_evaluator& m_evaluator; // The list of stack frames.