Ticket #4401 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

Segmentation fault in mc viewer (growbuf.c)

Reported by: misch Owned by: andrew_b
Priority: major Milestone: 4.8.29
Component: mcview Version: 4.8.26
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

The viewer crashes while viewing ELF binaries with hidden symbols, when switching from parse to raw mode and back. The parsed content is coming from nm via stdio pipe.

Steps to reproduce:

  1. Open binary without symbols in the viewer: mcview /usr/bin/ls
  2. Switch from parse to raw mode and back.

Debugging revealed that the crash is caused by double-closing the pipe:

  1. In src/viewer/growbuf.c:188 the output from nm is shown by mcview_show_error
  2. view->ds_stdio_pipe now suddenly is set to NULL, leaving sp invalid
  3. Line 213: mcview_growbuf_done (view); leads to:
  4. Line 82: which crashes in mc_pclose(NULL, NULL)

I suspect freeing resources down the messaging system triggers a second call to mcview_close_datasource -> mcview_growbuf_done.

As a working patch attached to this ticket I suggest to reintroduce the check that was removed by ticket #4103 (mc-4103-cid-growbuf.c-fix-dereference-before-null-check.patch) while taking sp's invalidity into account and to implement a NULL pointer check in mc_pclose as well.

Tested on 64-bit Ubuntu Linux and on ARMv7 Raspbian 10 (buster) which I also used for building and debugging MC, starting with the distro source package & build deps for MC 4.8.22. On Raspbian 11 (bullseye) the crash could be reproduced with MC 4.8.26. Afterwards I built and debugged the latest master from Git (commit 08cca8aae8c2f36e26ff99174f5cc40e551a98ab).

Version output:

# LC_MESSAGES=C src/mc -V

GNU Midnight Commander 4.8.28-86-g08cca8aae
Built with GLib 2.58.3
Built with S-Lang 2.3.2 with terminfo database
Built with libssh2 1.8.0
With builtin Editor and Aspell support
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
With ext2fs attributes support
Virtual File Systems:
 cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types:
 char: 8; int: 32; long: 32; void *: 32; size_t: 32; off_t: 64;

# LC_MESSAGES=C mc -F

Home directory: /home/pi
Profile root directory: /home/pi

[System data]
    Config directory: /etc/mc/
    Data directory:   /usr/share/mc/
    File extension handlers: /usr/lib/mc/ext.d/
    VFS plugins and scripts: /usr/lib/mc/
	extfs.d:        /usr/lib/mc/extfs.d/
	fish:           /usr/lib/mc/fish/

[User data]
    Config directory: /home/pi/.config/mc/
    Data directory:   /home/pi/.local/share/mc/
	skins:          /home/pi/.local/share/mc/skins/
	extfs.d:        /home/pi/.local/share/mc/extfs.d/
	fish:           /home/pi/.local/share/mc/fish/
	mcedit macros:  /home/pi/.local/share/mc/mc.macros
	mcedit external macros: /home/pi/.local/share/mc/mcedit/macros.d/macro.*
    Cache directory:  /home/pi/.cache/mc/

# mc --configure-options

'--build=arm-linux-gnueabihf'
'--prefix=/usr'
'--includedir=/usr/include'
'--mandir=/usr/share/man'
'--infodir=/usr/share/info'
'--sysconfdir=/etc'
'--localstatedir=/var'
'--disable-option-checking'
'--libdir=/usr/lib/arm-linux-gnueabihf'
'--libexecdir=/usr/lib/arm-linux-gnueabihf'
'--disable-maintainer-mode'
'--disable-dependency-tracking'
'AWK=awk'
'X11_WWW=x-www-browser'
'--libexecdir=/usr/lib'
'--with-x'
'--with-screen=slang'
'--disable-rpath'
'--disable-static'
'--disable-silent-rules'
'--enable-aspell'
'--enable-vfs-sftp'
'--enable-vfs-undelfs'
'build_alias=arm-linux-gnueabihf'

gdb Backtrace:

#0  0x000d8708 in mc_pclose (p=0x0, error=0x0) at utilunix.c:678
#1  0x000d1be0 in mcview_growbuf_done (view=0x226338) at growbuf.c:82
#2  0x000d2074 in mcview_growbuf_read_until (view=0x226338, ofs=1) at growbuf.c:213
#3  0x000d23d0 in mcview_get_ptr_growing_buffer (view=0x226338, byte_index=0) at growbuf.c:279
#4  0x000d2224 in mcview_get_byte_growing_buffer (view=0x226338, byte_index=0, retval=0x0) at growbuf.c:254
#5  0x000cf420 in mcview_get_byte (view=0x226338, offset=0, retval=0x0) at internal.h:399
#6  0x000cffbc in mcview_load_command_output (view=0x226338, command=0x2272d8 "/bin/sh /tmp/mc-pi/mcextJFYTQ1")
    at datasource.c:399
#7  0x000662cc in mcview_load (view=0x226338, command=0x2272d8 "/bin/sh /tmp/mc-pi/mcextJFYTQ1", 
    file=0x221d90 "/usr/bin/ls", start_line=0, search_start=0, search_end=0) at mcviewer.c:325
#8  0x00064af4 in mcview_toggle_magic_mode (view=0x226338) at lib.c:90
#9  0x000cbbbc in mcview_execute_cmd (view=0x226338, command=601) at actions_cmd.c:489
#10 0x000cc344 in mcview_callback (w=0x226338, sender=0x226588, msg=MSG_ACTION, parm=601, data=0x0)
    at actions_cmd.c:704
#11 0x00108e9c in send_message (w=0x226338, sender=0x226588, msg=MSG_ACTION, parm=601, data=0x0)
    at ../../lib/widget/widget-common.h:255
#12 0x00109358 in buttonbar_call (bb=0x226588, i=7) at buttonbar.c:154
#13 0x001093f0 in buttonbar_callback (w=0x226588, sender=0x0, msg=MSG_HOTKEY, parm=1008, data=0x0)
    at buttonbar.c:171
#14 0x0010c9c8 in send_message (w=0x226588, sender=0x0, msg=MSG_HOTKEY, parm=1008, data=0x0)
    at ../../lib/widget/widget-common.h:255
#15 0x0010dc54 in group_handle_hotkey (g=0x2254a0, key=1008) at group.c:564
#16 0x0010dec4 in group_default_callback (w=0x2254a0, sender=0x0, msg=MSG_HOTKEY, parm=1008, data=0x0)
    at group.c:638
#17 0x0010b4cc in dlg_default_callback (w=0x2254a0, sender=0x0, msg=MSG_HOTKEY, parm=1008, data=0x0)
    at dialog.c:368
#18 0x000cc538 in mcview_dialog_callback (w=0x2254a0, sender=0x0, msg=MSG_HOTKEY, parm=1008, data=0x0)
    at actions_cmd.c:785
#19 0x0010c9c8 in send_message (w=0x2254a0, sender=0x0, msg=MSG_HOTKEY, parm=1008, data=0x0)
    at ../../lib/widget/widget-common.h:255
#20 0x0010d9e0 in group_handle_key (g=0x2254a0, key=1008) at group.c:501
#21 0x0010deb0 in group_default_callback (w=0x2254a0, sender=0x0, msg=MSG_KEY, parm=1008, data=0x0) at group.c:635
#22 0x0010b4cc in dlg_default_callback (w=0x2254a0, sender=0x0, msg=MSG_KEY, parm=1008, data=0x0) at dialog.c:368
#23 0x000cc538 in mcview_dialog_callback (w=0x2254a0, sender=0x0, msg=MSG_KEY, parm=1008, data=0x0)
    at actions_cmd.c:785
#24 0x0010aae4 in send_message (w=0x2254a0, sender=0x0, msg=MSG_KEY, parm=1008, data=0x0)
    at ../../lib/widget/widget-common.h:255
#25 0x0010b120 in dlg_key_event (h=0x2254a0, d_key=1008) at dialog.c:248
#26 0x0010bd30 in dlg_process_event (h=0x2254a0, key=1008, event=0x7effee3c) at dialog.c:568
#27 0x0010b36c in frontend_dlg_run (h=0x2254a0) at dialog.c:320
#28 0x0010be30 in dlg_run (h=0x2254a0) at dialog.c:602
#29 0x000660bc in mcview_viewer (command=0x2071f0 "/bin/sh /tmp/mc-pi/mcextJFYTQ1", file_vpath=0x1ef460, 
    start_line=0, search_start=0, search_end=0) at mcviewer.c:266
#30 0x000260ec in exec_extension_view (target=0x0, cmd=0x2071f0 "/bin/sh /tmp/mc-pi/mcextJFYTQ1", 
    filename_vpath=0x1ef460, start_line=0) at ext.c:384
#31 0x00026630 in exec_extension (panel=0x1e7680, target=0x0, filename_vpath=0x1ef460, 
    lc_data=0x20d255 "%view{ascii} /usr/lib/mc/ext.d/misc.sh view elf\n\n### Documentation ###\n\n# Texinfo\n#regex/\\.(te?xi|texinfo)$\n\n# GNU Info page\ntype/^Info\\ text\n \tOpen=/usr/lib/mc/ext.d/text.sh open info\n\nshell/.info\n\tO"..., start_line=0) at ext.c:512
#32 0x000278d0 in regex_command_for (target=0x0, filename_vpath=0x1ef460, action=0x7efff094 "View", 
    script_vpath=0x0) at ext.c:1043
#33 0x000af380 in regex_command (filename_vpath=0x1ef460, action=0x7efff094 "View") at ext.h:30
#34 0x000b039c in view_file_at_line (filename_vpath=0x1ef460, plain_view=0, internal=1, start_line=0, 
    search_start=0, search_end=0) at cmd.c:532
#35 0x000b0528 in view_file (filename_vpath=0x1ef460, plain_view=0, internal=1) at cmd.c:571
#36 0x000af578 in do_view_cmd (panel=0x1e7680, plain_view=0) at cmd.c:153
#37 0x000b0554 in view_cmd (panel=0x1e7680) at cmd.c:581
#38 0x0002ecac in midnight_execute_cmd (sender=0x2024b8, command=101) at filemanager.c:1414
#39 0x0002f4a4 in midnight_callback (w=0x1ec8d8, sender=0x2024b8, msg=MSG_ACTION, parm=101, data=0x0)
    at filemanager.c:1609
#40 0x00108e9c in send_message (w=0x1ec8d8, sender=0x2024b8, msg=MSG_ACTION, parm=101, data=0x0)
    at ../../lib/widget/widget-common.h:255
#41 0x00109358 in buttonbar_call (bb=0x2024b8, i=2) at buttonbar.c:154
#42 0x001093f0 in buttonbar_callback (w=0x2024b8, sender=0x0, msg=MSG_HOTKEY, parm=1003, data=0x0)
    at buttonbar.c:171
#43 0x0010c9c8 in send_message (w=0x2024b8, sender=0x0, msg=MSG_HOTKEY, parm=1003, data=0x0)
    at ../../lib/widget/widget-common.h:255
#44 0x0010dc54 in group_handle_hotkey (g=0x1ec8d8, key=1003) at group.c:564
#45 0x0010dec4 in group_default_callback (w=0x1ec8d8, sender=0x0, msg=MSG_HOTKEY, parm=1003, data=0x0)
    at group.c:638
#46 0x0010b4cc in dlg_default_callback (w=0x1ec8d8, sender=0x0, msg=MSG_HOTKEY, parm=1003, data=0x0)
    at dialog.c:368
#47 0x0002f4d4 in midnight_callback (w=0x1ec8d8, sender=0x0, msg=MSG_HOTKEY, parm=1003, data=0x0)
    at filemanager.c:1616
#48 0x0010c9c8 in send_message (w=0x1ec8d8, sender=0x0, msg=MSG_HOTKEY, parm=1003, data=0x0)
    at ../../lib/widget/widget-common.h:255
#49 0x0010d9e0 in group_handle_key (g=0x1ec8d8, key=1003) at group.c:501
#50 0x0010deb0 in group_default_callback (w=0x1ec8d8, sender=0x0, msg=MSG_KEY, parm=1003, data=0x0) at group.c:635
#51 0x0010b14c in dlg_key_event (h=0x1ec8d8, d_key=1003) at dialog.c:251
#52 0x0010bd30 in dlg_process_event (h=0x1ec8d8, key=1003, event=0x7efff40c) at dialog.c:568
#53 0x0010b36c in frontend_dlg_run (h=0x1ec8d8) at dialog.c:320
#54 0x0010be30 in dlg_run (h=0x1ec8d8) at dialog.c:602
#55 0x0002feb0 in do_nc () at filemanager.c:1827
#56 0x00019458 in main (argc=2, argv=0x7efff624) at main.c:455

Attachments

mc-4401-gdb-bt-full.txt (10.7 KB) - added by misch 2 years ago.
gdb full backtrace
mc-4401-segmentation-fault-in-mcviewer.patch (2.1 KB) - added by misch 2 years ago.
working patch

Change History

Changed 2 years ago by misch

gdb full backtrace

Changed 2 years ago by misch

working patch

comment:1 Changed 2 years ago by andrew_b

  • Status changed from new to accepted
  • Owner set to andrew_b
  • Version changed from 4.8.28 to 4.8.26
  • Branch state changed from no branch to on review

Thanks!

Branch: 4401_viewer_segfault
changeset:a1f22e104decd691d4b0181ce2c01f03f595faa5

comment:2 Changed 2 years ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:3 Changed 2 years ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

comment:4 Changed 2 years ago by andrew_b

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.