Mercurial > octave
changeset 24071:3f036b46a130
avoid heap-use-after-free in callback execution (bug #52024)
* graphics.in.h, graphics.cc (callback_property::executing): Delete.
(callback_props): New class.
(executing_callbacks): Rename from executing_callback and declare as
callback_props object. Change all uses.
author | Pantxo Diribarne <pantxo.diribarne@gmail.com> |
---|---|
date | Thu, 21 Sep 2017 16:12:53 +0200 |
parents | 1091931bd63c |
children | dbbc7e5e2294 |
files | libinterp/corefcn/graphics.cc libinterp/corefcn/graphics.in.h |
diffstat | 2 files changed, 48 insertions(+), 20 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/graphics.cc Thu Sep 21 12:45:28 2017 -0400 +++ b/libinterp/corefcn/graphics.cc Thu Sep 21 16:12:53 2017 +0200 @@ -1766,28 +1766,59 @@ return false; } -// If TRUE, we are executing any callback function, or the functions it calls. -// Used to determine handle visibility inside callback functions. -static bool executing_callback = false; +class callback_props +{ +public: + + callback_props (void) : m_set () { } + + callback_props (const callback_props&) = delete; + + callback_props& operator = (const callback_props&) = delete; + + ~callback_props (void) = default; + + bool empty (void) const { return m_set.empty (); } + + void insert (const callback_property *ptr) + { + m_set.insert (reinterpret_cast<intptr_t> (ptr)); + } + + void erase (const callback_property *ptr) + { + m_set.erase (reinterpret_cast<intptr_t> (ptr)); + } + + bool contains (const callback_property *ptr) const + { + return m_set.find (reinterpret_cast<intptr_t> (ptr)) != m_set.end (); + } + +private: + + std::set<intptr_t> m_set; +}; + +// Elements of this set are pointers to currently executing +// callback_property objects. Used to determine handle visibility +// inside callback functions. + +static callback_props executing_callbacks; void callback_property::execute (const octave_value& data) const { octave::unwind_protect frame; - // We are executing the callback function associated with this - // callback property. When set to true, we avoid recursive calls to - // callback routines. - frame.protect_var (executing); - // We are executing a callback function, so allow handles that have // their handlevisibility property set to "callback" to be visible. - frame.protect_var (executing_callback); - - if (! executing) - { - executing = true; - executing_callback = true; + + frame.add_method (executing_callbacks, &callback_props::erase, this); + + if (! executing_callbacks.contains (this)) + { + executing_callbacks.insert (this); if (callback.is_defined () && ! callback.isempty ()) gh_manager::execute_callback (get_parent (), callback, data); @@ -3217,7 +3248,7 @@ base_properties::is_handle_visible (void) const { return (handlevisibility.is ("on") - || (executing_callback && ! handlevisibility.is ("off"))); + || (! executing_callbacks.empty () && ! handlevisibility.is ("off"))); } graphics_toolkit
--- a/libinterp/corefcn/graphics.in.h Thu Sep 21 12:45:28 2017 -0400 +++ b/libinterp/corefcn/graphics.in.h Thu Sep 21 16:12:53 2017 +0200 @@ -1891,10 +1891,10 @@ public: callback_property (const std::string& nm, const graphics_handle& h, const octave_value& m) - : base_property (nm, h), callback (m), executing (false) { } + : base_property (nm, h), callback (m) { } callback_property (const callback_property& p) - : base_property (p), callback (p.callback), executing (false) { } + : base_property (p), callback (p.callback) { } octave_value get (void) const { return callback; } @@ -1930,9 +1930,6 @@ private: octave_value callback; - - // If TRUE, we are executing this callback. - mutable bool executing; }; // ---------------------------------------------------------------------