Ticket #3637 (closed defect: fixed)

Opened 16 months ago

Last modified 15 months ago

[PATCH] Fix memleak at backwards search

Reported by: and Owned by: andrew_b
Priority: major Milestone: 4.8.18
Component: mc-search Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

Fix memleak at backwards search when not found result happen.

Attachments

mc-3637-fix-memleak-at-backwards-search.patch (6.6 KB) - added by and 16 months ago.

Change History

Changed 16 months ago by and

comment:1 Changed 16 months ago by andrew_b

It seems, there are memleaks in editor in case of search "In selection": editcmd.c:832 and editcmd.c:853.

comment:2 Changed 16 months ago by andrew_b

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

comment:3 follow-up: ↓ 4 Changed 16 months ago by and

Mhh, I can't provoke any "in selection" memleaks cases, but this additional issue I have found:

-> open long text file with editor (to get a change to see search menu and abort button)
-> jump to middle or end of the text file
-> select "Normal" + "Backwards"
-> search for a non-existing string
-> hit abort and wait

use-after-free will occur on backwards + hit abort button only(?).

(this stacktrace include #3637 patch already in case of different code lines)

==18636==ERROR: AddressSanitizer: heap-use-after-free on address 0x6100000026b8 at pc 0x7fe7f0a42242 bp 0x7ffde9fc8630 sp 0x7ffde9fc8628
READ of size 4 at 0x6100000026b8 thread T0
    #0 0x7fe7f0a42241 in do_refresh /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1023:54
    #1 0x661a60 in edit_do_search /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editcmd.c:1028:5
    #2 0x630bd5 in edit_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/edit.c:3812:9
    #3 0x62f451 in edit_execute_key_command /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/edit.c:3218:5
    #4 0x689575 in edit_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:965:17
    #5 0x7fe7f0a38327 in buttonbar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/buttonbar.c:174:42
    #6 0x7fe7f0a46a45 in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:480:23
    #7 0x7fe7f0a44e4e in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:525:19
    #8 0x7fe7f0a45730 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:586:9
    #9 0x7fe7f0a45345 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1300:5
    #10 0x6872b8 in edit_files /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:1250:9
    #11 0x68689f in edit_file /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:1189:10
    #12 0x6d493b in edit_file_at_line /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:703:9
    #13 0x6d4b2d in edit_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:741:9
    #14 0x55b1a5 in midnight_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1173:9
    #15 0x7fe7f0a38327 in buttonbar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/buttonbar.c:174:42
    #16 0x7fe7f0a46a45 in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:480:23
    #17 0x7fe7f0a44e4e in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:525:19
    #18 0x7fe7f0a45730 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:586:9
    #19 0x7fe7f0a45345 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1300:5
    #20 0x558d97 in do_nc /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1776:9
    #21 0x510595 in main /tmp/portage/app-misc/mc-9999/work/mc-9999/src/main.c:403:21
    #22 0x7fe7ef743733 in __libc_start_main (/lib64/libc.so.6+0x20733)
    #23 0x427ea8 in _start (/usr/bin/mc+0x427ea8)

0x6100000026b8 is located 120 bytes inside of 192-byte region [0x610000002640,0x610000002700)
freed by thread T0 here:
    #0 0x4d8c30 in __interceptor_free (/usr/bin/mc+0x4d8c30)
    #1 0x7fe7f0a45aac in dlg_destroy /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1317:5
    #2 0x661a60 in edit_do_search /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editcmd.c:1028:5
    #3 0x630bd5 in edit_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/edit.c:3812:9
    #4 0x62f451 in edit_execute_key_command /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/edit.c:3218:5
    #5 0x689575 in edit_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:965:17
    #6 0x7fe7f0a38327 in buttonbar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/buttonbar.c:174:42
    #7 0x7fe7f0a46a45 in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:480:23
    #8 0x7fe7f0a44e4e in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:525:19
    #9 0x7fe7f0a45730 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:586:9
    #10 0x7fe7f0a45345 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1300:5
    #11 0x6872b8 in edit_files /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:1250:9
    #12 0x68689f in edit_file /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:1189:10
    #13 0x6d493b in edit_file_at_line /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:703:9
    #14 0x6d4b2d in edit_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:741:9
    #15 0x55b1a5 in midnight_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1173:9
    #16 0x7fe7f0a38327 in buttonbar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/buttonbar.c:174:42
    #17 0x7fe7f0a46a45 in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:480:23
    #18 0x7fe7f0a44e4e in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:525:19
    #19 0x7fe7f0a45730 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:586:9
    #20 0x7fe7f0a45345 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1300:5
    #21 0x558d97 in do_nc /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1776:9
    #22 0x510595 in main /tmp/portage/app-misc/mc-9999/work/mc-9999/src/main.c:403:21
    #23 0x7fe7ef743733 in __libc_start_main (/lib64/libc.so.6+0x20733)
    #24 0x427ea8 in _start (/usr/bin/mc+0x427ea8)

previously allocated by thread T0 here:
    #0 0x4d9130 in __interceptor_calloc (/usr/bin/mc+0x4d9130)
    #1 0x7fe7f0cd032b in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x9b32b)
    #2 0x7fe7f0a3f02d in dlg_create /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:822:13
    #3 0x7fe7f0aa0996 in status_msg_init /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/wtools.c:588:15
    #4 0x660c54 in edit_do_search /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editcmd.c:966:5
    #5 0x630bd5 in edit_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/edit.c:3812:9
    #6 0x62f451 in edit_execute_key_command /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/edit.c:3218:5
    #7 0x689575 in edit_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:965:17
    #8 0x7fe7f0a38327 in buttonbar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/buttonbar.c:174:42
    #9 0x7fe7f0a46a45 in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:480:23
    #10 0x7fe7f0a44e4e in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:525:19
    #11 0x7fe7f0a45730 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:586:9
    #12 0x7fe7f0a45345 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1300:5
    #13 0x6872b8 in edit_files /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:1250:9
    #14 0x68689f in edit_file /tmp/portage/app-misc/mc-9999/work/mc-9999/src/editor/editwidget.c:1189:10
    #15 0x6d493b in edit_file_at_line /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:703:9
    #16 0x6d4b2d in edit_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:741:9
    #17 0x55b1a5 in midnight_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1173:9
    #18 0x7fe7f0a38327 in buttonbar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/buttonbar.c:174:42
    #19 0x7fe7f0a46a45 in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:480:23
    #20 0x7fe7f0a44e4e in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:525:19
    #21 0x7fe7f0a45730 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:586:9
    #22 0x7fe7f0a45345 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1300:5
    #23 0x558d97 in do_nc /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1776:9
    #24 0x510595 in main /tmp/portage/app-misc/mc-9999/work/mc-9999/src/main.c:403:21
    #25 0x7fe7ef743733 in __libc_start_main (/lib64/libc.so.6+0x20733)
    #26 0x427ea8 in _start (/usr/bin/mc+0x427ea8)

comment:4 in reply to: ↑ 3 Changed 16 months ago by andrew_b

Replying to and:

Actually, we have two bugs here: pressing "Abort" doesn't interrupt backward search (probably, separate ticket?) and after pressing we have segfault after search is finished.

comment:5 Changed 16 months ago by andrew_b

  • Branch state changed from on review to on rework

comment:6 follow-up: ↓ 7 Changed 16 months ago by zaytsev

Hi Andrew, so shall we push this to 4.7.18 or you definitively want to finish this for the next release within the next couple of days? Thanks!

comment:7 in reply to: ↑ 6 Changed 16 months ago by andrew_b

  • Milestone changed from 4.8.17 to 4.8.18

Replying to zaytsev:

Hi Andrew, so shall we push this to 4.7.18

Yep. This is more complicated than I expected and I don't want introduce new bugs.

comment:8 Changed 16 months ago by andrew_b

  • Branch state changed from on rework to on review

Branch: 3637_search_memleaks
Initial changeset:0744a155a1c3f6afd2f68b6e59fa454c90d6d379

comment:9 Changed 15 months ago by andrew_b

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

comment:10 Changed 15 months 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

Merged to master: [35ad1ab0b27530779c3dfd7d6ec0443527283dce].

git log --pretty=oneline 9cd1e16..35ad1ab

comment:11 Changed 15 months ago by andrew_b

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