bugGNU Octave - Bugs: bug #52024, Memory issue using File->Close...

 
 

bug #52024: Memory issue using File->Close from FLTK figure window

Submitter:  Rik <rik5>
Submitted:  Thu 14 Sep 2017 07:59:25 PM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 21 Sep 2017 06:44:50 PM UTC, comment #5: 

Tested the cset and it works just fine.  Marking as fixed and closing report.

Rik <rik5>
Group administrator
Thu 21 Sep 2017 05:57:36 PM UTC, comment #4: 

Casting to intptr_t seems OK to me, but I decided to hide that in a custom class and use insert instead of emplace and add a member function callback to the unwind-protect stack instead of attempting to restore the value of the std::set variable.

I pushed this modified version of your changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/3f036b46a130

It seems to work for me but I'll wait for confirmation and any additional comments before closing.

John W. Eaton <jwe>
Group administrator
Thu 21 Sep 2017 02:26:34 PM UTC, comment #3: 

In an unwind_protect object, I don't think there is a way to know if the pointer to a variable is pointing to a freed heap memory or not. If I am not wrong on that point, it is not safe to use unwind_protect objects to restore variables that we can't be sure will still be allocated at restore time.

I attached a tentative patch that calls for criticism since (i) I am not sure it is good practice to cast pointers to int, (ii) I don't know how to check things still work as expected, i.e. a callback can't be executed recursively.



(file #41854)

Pantxo Diribarne <pantxo>
Group Member
Wed 20 Sep 2017 02:45:21 PM UTC, comment #2: 

If I do the following change I don't see the heap-use-after-free anymore:


diff -r 83cd00e6582b libinterp/corefcn/graphics.cc
--- a/libinterp/corefcn/graphics.cc        Wed Sep 20 16:07:51 2017 +0200
+++ b/libinterp/corefcn/graphics.cc        Wed Sep 20 16:42:06 2017 +0200
@@ -1778,7 +1778,7 @@
   // 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);
+  //frame.protect_var (executing);

   // We are executing a callback function, so allow handles that have
   // their handlevisibility property set to "callback" to be visible.


The issue here is that "executing" is an attribute of the callback_property object which will be destroyed by the callback execution and will thus be already freed when the unwind_protect object tries to restore its value.


Pantxo Diribarne <pantxo>
Group Member
Tue 19 Sep 2017 06:12:21 PM UTC, comment #1: 

The issue is still not completely understood, but for a new FLTK figure the function _add_default_menu_.m is called.  This creates the uimenu object for the figure.  One of the menu entries is File->Close and the associated callback is the pure piece of Octave code "close (gcf)".

The issue seems to be that FLTK detects the menu click, and then the uimenu object attempts to run the callback code in an unwind/protect block.  The callback code closes the figure which deletes the uimenu object.  When control returns from the callback, the unwind protect block now has a non-existent uimenu object.  That's all I can tell so far.  I don't have a good way of solving this just yet.

Rik <rik5>
Group administrator
Thu 14 Sep 2017 07:59:25 PM UTC, original submission:  

Creating a figure and then closing it with "File->Close" creates a use-after-free error.  If the figure is closed from the command line using "close" then there is no such problem.

Sample code:


./run-octave -f --no-gui
plot (1:10)
# Now execute File->Close


The backtrace is


==25931==ERROR: AddressSanitizer: heap-use-after-free on address 0x6220000340b0 at pc 0x7f8287e4b42a bp 0x7ffcaa7007a0 sp 0x7ffcaa700790
WRITE of size 1 at 0x6220000340b0 thread T0
    #0 0x7f8287e4b429 in octave::action_container::restore_var_elem<bool>::run() liboctave/util/action-container.h:317
    #1 0x7f8287d1d1b2 in octave::unwind_protect::run_first() liboctave/util/unwind-prot.h:72
    #2 0x7f8287d1cd22 in octave::action_container::run(unsigned long) liboctave/util/action-container.h:477
    #3 0x7f8287d1cda3 in octave::action_container::run() liboctave/util/action-container.h:480
    #4 0x7f8287d1cfca in octave::unwind_protect::~unwind_protect() liboctave/util/unwind-prot.h:56
    #5 0x7f8288285569 in callback_property::execute(octave_value const&) const libinterp/corefcn/graphics.cc:1776
    #6 0x7f8270c7eb4b in uimenu::properties::execute_callback(octave_value const&) const libinterp/corefcn/graphics.h:11505
    #7 0x7f8270c7128e in script_cb(Fl_Widget*, void*) libinterp/dldfcn/__init_fltk__.cc:311
    #8 0x7f827072236a in Fl_Menu_::picked(Fl_Menu_Item const*) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x6c36a)
    #9 0x7f8270722d3e in Fl_Menu_Bar::handle(int) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x6cd3e)
    #10 0x7f827070e372 in Fl_Group::handle(int) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x58372)
    #11 0x7f8270c912a4 in plot_window::handle(int) libinterp/dldfcn/__init_fltk__.cc:1854
    #12 0x7f82706f75ac  (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x415ac)
    #13 0x7f82706f9164 in Fl::handle_(int, Fl_Window*) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x43164)
    #14 0x7f8270754e69 in fl_handle(_XEvent const&) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x9ee69)
    #15 0x7f8270755fd1  (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x9ffd1)
    #16 0x7f8270756483 in fl_wait(double) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0xa0483)
    #17 0x7f82706f8c0d in Fl::wait(double) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x42c0d)
    #18 0x7f82706f8d5c in Fl::check() (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x42d5c)
    #19 0x7f8270c71688 in F__fltk_check__(octave_value_list const&, int) libinterp/dldfcn/__init_fltk__.cc:2475
    #20 0x7f8287d1c3fe in octave_builtin::call(octave::tree_evaluator&, int, octave_value_list const&) libinterp/octave-value/ov-builtin.cc:65
    #21 0x7f8287fd1243 in octave::feval(octave_function*, octave_value_list const&, int) libinterp/parse-tree/oct-parse.yy:5145
    #22 0x7f8287fd1438 in octave::feval(octave_value&, octave_value_list const&, int) libinterp/parse-tree/oct-parse.yy:5156
    #23 0x7f8287def000 in octave_fcn_handle::call(int, octave_value_list const&) libinterp/octave-value/ov-fcn-handle.cc:216
    #24 0x7f8287dee660 in octave_fcn_handle::subsref(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::list<octave_value_list, std::allocator<octave_value_list> > const&, int) libinterp/octave-value/ov-fcn-handle.cc:112
    #25 0x7f8287ea78cf in octave_value::subsref(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::list<octave_value_list, std::allocator<octave_value_list> > const&, int) libinterp/octave-value/ov.cc:1423
    #26 0x7f8287fd14cf in octave::feval(octave_value&, octave_value_list const&, int) libinterp/parse-tree/oct-parse.yy:5166
    #27 0x7f828848c5ba in fcn_handle_hook_function::eval(octave_value_list const&) libinterp/corefcn/hook-fcn.h:172
    #28 0x7f8288493734 in hook_function::eval(octave_value_list const&) libinterp/corefcn/hook-fcn.h:107
    #29 0x7f8288493a0b in hook_function_list::run(octave_value_list const&) libinterp/corefcn/hook-fcn.h:253
    #30 0x7f8288491f07 in internal_input_event_hook_fcn libinterp/corefcn/input.cc:1125
    #31 0x7f8286987ed4 in octave::command_editor::event_handler() liboctave/util/cmd-edit.cc:1143
    #32 0x7f827ff5e1c3 in rl_read_key (/lib/x86_64-linux-gnu/libreadline.so.6+0x2a1c3)
    #33 0x7f827ff47dd1 in readline_internal_char (/lib/x86_64-linux-gnu/libreadline.so.6+0x13dd1)
    #34 0x7f827ff48544 in readline (/lib/x86_64-linux-gnu/libreadline.so.6+0x14544)
    #35 0x7f8286a03a27 in octave_rl_readline liboctave/util/oct-rl-edit.c:215
    #36 0x7f828698529b in octave::gnu_readline::do_readline(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool&) liboctave/util/cmd-edit.cc:292
    #37 0x7f8286988295 in octave::command_editor::readline(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool&) liboctave/util/cmd-edit.cc:1174
    #38 0x7f828848ca98 in gnu_readline libinterp/corefcn/input.cc:144
    #39 0x7f828848cd01 in interactive_input libinterp/corefcn/input.cc:189
    #40 0x7f828848d122 in octave::base_reader::octave_gets[abi:cxx11](bool&) libinterp/corefcn/input.cc:231
    #41 0x7f828848fcf5 in octave::terminal_reader::get_input[abi:cxx11](bool&) libinterp/corefcn/input.cc:708
    #42 0x7f8287fa10a8 in octave::input_reader::get_input[abi:cxx11](bool&) libinterp/corefcn/input.h:255
    #43 0x7f8287fa0187 in octave::lexer::fill_flex_buffer(char*, unsigned int) libinterp/parse-tree/lex.ll:3655
    #44 0x7f8287f90a1b in yy_get_next_buffer libinterp/parse-tree/lex.cc:3451
    #45 0x7f8287f8f742 in octave_lex(OCTAVE_STYPE*, void*) libinterp/parse-tree/lex.cc:3291
    #46 0x7f8287faa972 in octave_pull_parse(octave_pstate*, octave::base_parser&) libinterp/parse-tree/oct-parse.cc:2992
    #47 0x7f8287fcb17b in octave::parser::run() libinterp/parse-tree/oct-parse.yy:4314
    #48 0x7f828849f6a7 in octave::interpreter::main_loop() libinterp/corefcn/interpreter.cc:968
    #49 0x7f828849d577 in octave::interpreter::execute() libinterp/corefcn/interpreter.cc:695
    #50 0x7f82877f43eb in octave::cli_application::execute() libinterp/octave.cc:384
    #51 0x401d7c in main src/main-cli.cc:90
    #52 0x7f8284f7982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #53 0x401808 in _start (/home/rik/wip/Projects_Mine/octave-dbg/src/.libs/lt-octave-cli+0x401808)

0x6220000340b0 is located 4016 bytes inside of 5416-byte region [0x622000033100,0x622000034628)
freed by thread T0 here:
    #0 0x7f8288fb6b2a in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99b2a)
    #1 0x7f828847136f in uimenu::~uimenu() libinterp/corefcn/graphics.h:11645
    #2 0x7f828825e8f2 in graphics_object::~graphics_object() libinterp/corefcn/graphics.h:2929
    #3 0x7f828826b00f in std::_List_node<graphics_object>::~_List_node() /usr/include/c++/5/bits/stl_list.h:106
    #4 0x7f828826b02f in void __gnu_cxx::new_allocator<std::_List_node<graphics_object> >::destroy<std::_List_node<graphics_object> >(std::_List_node<graphics_object>*) /usr/include/c++/5/ext/new_allocator.h:124
    #5 0x7f828826a928 in std::__cxx11::list<graphics_object, std::allocator<graphics_object> >::_M_erase(std::_List_iterator<graphics_object>) /usr/include/c++/5/bits/stl_list.h:1777
    #6 0x7f8288450c99 in std::__cxx11::list<graphics_object, std::allocator<graphics_object> >::pop_front() /usr/include/c++/5/bits/stl_list.h:1075
    #7 0x7f82883e793a in gh_manager::do_restore_gcbo() libinterp/corefcn/graphics.cc:10091
    #8 0x7f8288448bb7 in gh_manager::restore_gcbo() libinterp/corefcn/graphics.h:14184
    #9 0x7f82883fb15e in octave::action_container::fcn_elem::run() liboctave/util/action-container.h:74
    #10 0x7f8287d1d1b2 in octave::unwind_protect::run_first() liboctave/util/unwind-prot.h:72
    #11 0x7f8287dfbd73 in octave::unwind_protect_safe::~unwind_protect_safe() liboctave/util/unwind-prot.h:121
    #12 0x7f82883e862f in gh_manager::do_execute_callback(octave_handle const&, octave_value const&, octave_value const&) libinterp/corefcn/graphics.cc:10127
    #13 0x7f82884483f6 in gh_manager::execute_callback(octave_handle const&, octave_value const&, octave_value const&) libinterp/corefcn/graphics.h:13962
    #14 0x7f828828555d in callback_property::execute(octave_value const&) const libinterp/corefcn/graphics.cc:1793
    #15 0x7f8270c7eb4b in uimenu::properties::execute_callback(octave_value const&) const libinterp/corefcn/graphics.h:11505
    #16 0x7f8270c7128e in script_cb(Fl_Widget*, void*) libinterp/dldfcn/__init_fltk__.cc:311
    #17 0x7f827072236a in Fl_Menu_::picked(Fl_Menu_Item const*) (/usr/lib/x86_64-linux-gnu/libfltk.so.1.3+0x6c36a)

previously allocated by thread T0 here:
    #0 0x7f8288fb6532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x7f828827dbb0 in make_graphics_object_from_type libinterp/corefcn/graphics.cc:1135
    #2 0x7f82883e6aba in gh_manager::do_make_graphics_handle(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, octave_handle const&, bool, bool, bool) libinterp/corefcn/graphics.cc:9839
    #3 0x7f8288447e42 in gh_manager::make_graphics_handle(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, octave_handle const&, bool, bool, bool) libinterp/corefcn/graphics.h:13870
    #4 0x7f82883ee7e2 in make_graphics_object libinterp/corefcn/graphics.cc:10988
    #5 0x7f82883f139c in F__go_uimenu__(octave_value_list const&, int) libinterp/corefcn/graphics.cc:11235
    #6 0x7f8287d1c3fe in octave_builtin::call(octave::tree_evaluator&, int, octave_value_list const&) libinterp/octave-value/ov-builtin.cc:65
    #7 0x7f8288011aa1 in octave::tree_evaluator::visit_index_expression(octave::tree_index_expression&) libinterp/parse-tree/pt-eval.cc:1252
    #8 0x7f82880449ec in octave::tree_index_expression::accept(octave::tree_walker&) libinterp/parse-tree/pt-idx.h:101
    #9 0x7f8287d9ce92 in octave::tree_evaluator::evaluate(octave::tree_expression*, int) libinterp/parse-tree/pt-eval.h:271
    #10 0x7f828801724a in octave::tree_evaluator::visit_simple_assignment(octave::tree_simple_assignment&) libinterp/parse-tree/pt-eval.cc:2085
    #11 0x7f8287ffca16 in octave::tree_simple_assignment::accept(octave::tree_walker&) libinterp/parse-tree/pt-assign.h:83
    #12 0x7f8287d9ce92 in octave::tree_evaluator::evaluate(octave::tree_expression*, int) libinterp/parse-tree/pt-eval.h:271
    #13 0x7f8288018164 in octave::tree_evaluator::visit_statement(octave::tree_statement&) libinterp/parse-tree/pt-eval.cc:2209
    #14 0x7f828805613e in octave::tree_statement::accept(octave::tree_walker&) libinterp/parse-tree/pt-stmt.h:112
    #15 0x7f828801847f in octave::tree_evaluator::visit_statement_list(octave::tree_statement_list&) libinterp/parse-tree/pt-eval.cc:2251
    #16 0x7f8287d9d730 in octave::tree_statement_list::accept(octave::tree_walker&) libinterp/parse-tree/pt-stmt.h:187
    #17 0x7f8287e9514d in octave_user_function::call(octave::tree_evaluator&, int, octave_value_list const&) libinterp/octave-value/ov-usr-fcn.cc:647
    #18 0x7f8288011aa1 in octave::tree_evaluator::visit_index_expression(octave::tree_index_expression&) libinterp/parse-tree/pt-eval.cc:1252
    #19 0x7f82880449ec in octave::tree_index_expression::accept(octave::tree_walker&) libinterp/parse-tree/pt-idx.h:101
    #20 0x7f8287d9ce92 in octave::tree_evaluator::evaluate(octave::tree_expression*, int) libinterp/parse-tree/pt-eval.h:271
    #21 0x7f8288018164 in octave::tree_evaluator::visit_statement(octave::tree_statement&) libinterp/parse-tree/pt-eval.cc:2209
    #22 0x7f828805613e in octave::tree_statement::accept(octave::tree_walker&) libinterp/parse-tree/pt-stmt.h:112
    #23 0x7f828801847f in octave::tree_evaluator::visit_statement_list(octave::tree_statement_list&) libinterp/parse-tree/pt-eval.cc:2251
    #24 0x7f8287d9d730 in octave::tree_statement_list::accept(octave::tree_walker&) libinterp/parse-tree/pt-stmt.h:187
    #25 0x7f8288010cbc in octave::tree_evaluator::visit_if_command_list(octave::tree_if_command_list&) libinterp/parse-tree/pt-eval.cc:1073
    #26 0x7f828800047e in octave::tree_if_command_list::accept(octave::tree_walker&) libinterp/parse-tree/pt-select.h:116
    #27 0x7f8288010a69 in octave::tree_evaluator::visit_if_command(octave::tree_if_command&) libinterp/parse-tree/pt-eval.cc:1052
    #28 0x7f8288053914 in octave::tree_if_command::accept(octave::tree_walker&) libinterp/parse-tree/pt-select.h:149
    #29 0x7f8288017e6f in octave::tree_evaluator::visit_statement(octave::tree_statement&) libinterp/parse-tree/pt-eval.cc:2176

SUMMARY: AddressSanitizer: heap-use-after-free liboctave/util/action-container.h:317 octave::action_container::restore_var_elem<bool>::run()
Shadow bytes around the buggy address:
  0x0c447fffe7c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe7d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe7e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe7f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe800: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c447fffe810: fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd
  0x0c447fffe820: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe830: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe840: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe850: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c447fffe860: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
==25931==ABORTING



Rik <rik5>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #41854:  safe_callback_exec.patch added by pantxo (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by rik5 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-09-21 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2017-09-21 jwe StatusConfirmed Ready For Test
    2017-09-21 pantxo Attached File- Added safe_callback_exec.patch, #41854
    2017-09-20 pantxo StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code