From 58229b6eacaddcd7ceec8e45a52fba6d3993e700 Mon Sep 17 00:00:00 2001
From: Andreas Mohr <and@gmx.li>
Date: Sun, 3 Jan 2016 15:08:19 +0000
Subject: [PATCH] sftp: fix connection memleaks
1) open_connection abort not handled correct yet
2) missing libssh2_session_free() on close_connection
3) internal list from libssh2_userauth_list() must not freed by mc
4) cosmetic: re-order open_connection and close_connection steps
5) cosmetic: sftpfs_super_data created at sftpfs_cb_open_connection() was freed at sftpfs_close_connection()
it should be sftpfs_cb_close_connection() for logical right location
6) add FIXME for deprected libssh2_session_startup() since libssh2 1.2.8
found by Clang/AddressSanitizer
Direct leak of 54520 byte(s) in 1 object(s) allocated from:
#0 0x4c7310 in __interceptor_malloc (/tmp/portage/app-misc/mc-9999/work/mc-9999/src/.libs/mc+0x4c7310)
#1 0x7fceea957200 in libssh2_session_init_ex (/usr/lib64/libssh2.so.1+0x15200)
#2 0x620c2e in sftpfs_open_connection /tmp/portage/app-misc/mc-9999/work/mc-9999/src/vfs/sftpfs/connection.c:372:27
#3 0x620c2e in sftpfs_cb_open_connection /tmp/portage/app-misc/mc-9999/work/mc-9999/src/vfs/sftpfs/vfs_subclass.c:123
#4 0x7fceeb19ff59 in vfs_s_get_path /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:1139:18
#5 0x7fceeb1a513d in vfs_s_inode_from_path /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:380:9
#6 0x7fceeb1a3727 in vfs_s_opendir /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:409:11
#7 0x7fceeb1a3c88 in vfs_s_chdir /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:476:12
#8 0x7fceeb1a87c3 in mc_chdir /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/interface.c:687:14
#9 0x535b44 in _do_panel_cd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/panel.c:3250:9
#10 0x5f37e1 in do_panel_cd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/panel.c:4627:9
#11 0x5f37e1 in nice_cd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:460
#12 0x529961 in fishlink_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:1430:5
#13 0x529961 in midnight_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1208
#14 0x7fceeb1d909d in send_message /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/../../lib/widget/widget-common.h:167:15
#15 0x7fceeb1d909d in menubar_execute /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/menu.c:341
#16 0x7fceeb1d830b in menubar_handle_key /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/menu.c:539:13
#17 0x7fceeb1d555f in menubar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/menu.c:597:13
#18 0x7fceeb1bbfec in send_message /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/../../lib/widget/widget-common.h:167:15
#19 0x7fceeb1bbfec in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:450
#20 0x7fceeb1bbfec in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:509
#21 0x7fceeb1bbfec in dlg_process_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1236
#22 0x7fceeb1bd9e7 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:570:9
#23 0x7fceeb1bc585 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1267:5
#24 0x4fc7b8 in create_panels_and_run_mc /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:954:5
#25 0x4fc7b8 in do_nc /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1757
#26 0x4fc7b8 in main /tmp/portage/app-misc/mc-9999/work/mc-9999/src/main.c:401
#27 0x7fcee9722953 in __libc_start_main (/lib64/libc.so.6+0x20953)
#28 0x4270e8 in _start (/tmp/portage/app-misc/mc-9999/work/mc-9999/src/.libs/mc+0x4270e8)
Indirect leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x4c7370 in __interceptor_malloc (/tmp/portage/app-misc/mc-9999/work/mc-9999/src/.libs/mc+0x4c7370)
#1 0x7feb83ffa366 in CRYPTO_malloc (/usr/lib64/libcrypto.so.1.0.0+0x1a8366)
#2 0x7feb83ee7316 (/usr/lib64/libcrypto.so.1.0.0+0x95316)
#3 0x7feb83ee7ae7 in bn_expand2 (/usr/lib64/libcrypto.so.1.0.0+0x95ae7)
#4 0x7feb83ee817e in BN_bin2bn (/usr/lib64/libcrypto.so.1.0.0+0x9617e)
#5 0x7feb85b80100 (/usr/lib64/libssh2.so.1+0x25100)
#6 0x7feb85b65d9f (/usr/lib64/libssh2.so.1+0xad9f)
#7 0x7feb85b664d0 (/usr/lib64/libssh2.so.1+0xb4d0)
#8 0x7feb85b67a5c (/usr/lib64/libssh2.so.1+0xca5c)
#9 0x7feb85b68756 (/usr/lib64/libssh2.so.1+0xd756)
#10 0x7feb85b71563 in libssh2_session_handshake (/usr/lib64/libssh2.so.1+0x16563)
#11 0x620dc1 in sftpfs_open_connection /tmp/portage/app-misc/mc-9999/work/mc-9999/src/vfs/sftpfs/connection.c:389:10
#12 0x620dc1 in sftpfs_cb_open_connection /tmp/portage/app-misc/mc-9999/work/mc-9999/src/vfs/sftpfs/vfs_subclass.c:123
#13 0x7feb863b8f29 in vfs_s_get_path /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:1139:18
#14 0x7feb863be10d in vfs_s_inode_from_path /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:380:9
#15 0x7feb863bc6f7 in vfs_s_opendir /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:409:11
#16 0x7feb863bcc58 in vfs_s_chdir /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/direntry.c:476:12
#17 0x7feb863c1793 in mc_chdir /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/vfs/interface.c:687:14
#18 0x535bb4 in _do_panel_cd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/panel.c:3250:9
#19 0x5f3891 in do_panel_cd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/panel.c:4627:9
#20 0x5f3891 in nice_cd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:460
#21 0x5299d1 in fishlink_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/cmd.c:1430:5
#22 0x5299d1 in midnight_execute_cmd /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1208
#23 0x7feb863f207d in send_message /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/../../lib/widget/widget-common.h:167:15
#24 0x7feb863f207d in menubar_execute /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/menu.c:341
#25 0x7feb863f12eb in menubar_handle_key /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/menu.c:539:13
#26 0x7feb863ee53f in menubar_callback /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/menu.c:597:13
#27 0x7feb863d4fcc in send_message /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/../../lib/widget/widget-common.h:167:15
#28 0x7feb863d4fcc in dlg_try_hotkey /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:450
#29 0x7feb863d4fcc in dlg_key_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:509
#30 0x7feb863d4fcc in dlg_process_event /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1236
#31 0x7feb863d69c7 in frontend_dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:570:9
#32 0x7feb863d5565 in dlg_run /tmp/portage/app-misc/mc-9999/work/mc-9999/lib/widget/dialog.c:1267:5
#33 0x4fc818 in create_panels_and_run_mc /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:954:5
#34 0x4fc818 in do_nc /tmp/portage/app-misc/mc-9999/work/mc-9999/src/filemanager/midnight.c:1757
#35 0x4fc818 in main /tmp/portage/app-misc/mc-9999/work/mc-9999/src/main.c:401
#36 0x7feb8493b953 in __libc_start_main (/lib64/libc.so.6+0x20953)
#37 0x427148 in _start (/tmp/portage/app-misc/mc-9999/work/mc-9999/src/.libs/mc+0x427148)
Signed-off-by: Andreas Mohr <and@gmx.li>
---
src/vfs/sftpfs/connection.c | 37 +++++++++++++++++++++----------------
src/vfs/sftpfs/vfs_subclass.c | 6 ++++++
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/src/vfs/sftpfs/connection.c b/src/vfs/sftpfs/connection.c
index fceb961..ba60076 100644
a
|
b
|
sftpfs_recognize_auth_types (struct vfs_s_super *super) |
170 | 170 | super_data->auth_type = NONE; |
171 | 171 | |
172 | 172 | /* check what authentication methods are available */ |
| 173 | /* userauthlist is internally managed by libssh2 and |
| 174 | freed by libssh2_session_free() */ |
173 | 175 | userauthlist = libssh2_userauth_list (super_data->session, super->path_element->user, |
174 | 176 | strlen (super->path_element->user)); |
175 | 177 | |
… |
… |
sftpfs_recognize_auth_types (struct vfs_s_super *super) |
183 | 185 | |
184 | 186 | if ((super_data->config_auth_type & AGENT) != 0) |
185 | 187 | super_data->auth_type |= AGENT; |
186 | | |
187 | | g_free (userauthlist); |
188 | 188 | } |
189 | 189 | |
190 | 190 | /* --------------------------------------------------------------------------------------------- */ |
… |
… |
sftpfs_open_connection (struct vfs_s_super *super, GError ** mcerror) |
368 | 368 | |
369 | 369 | super_data = (sftpfs_super_data_t *) super->data; |
370 | 370 | |
371 | | /* Create a session instance */ |
372 | | super_data->session = libssh2_session_init (); |
373 | | if (super_data->session == NULL) |
374 | | return (-1); |
| 371 | super_data->session = NULL; |
| 372 | super_data->agent = NULL; |
| 373 | super_data->sftp_session = NULL; |
375 | 374 | |
376 | 375 | /* |
377 | 376 | * The application code is responsible for creating the socket |
… |
… |
sftpfs_open_connection (struct vfs_s_super *super, GError ** mcerror) |
381 | 380 | if (super_data->socket_handle == -1) |
382 | 381 | return (-1); |
383 | 382 | |
| 383 | /* Create a session instance */ |
| 384 | super_data->session = libssh2_session_init (); |
| 385 | if (super_data->session == NULL) |
| 386 | return (-1); |
| 387 | |
384 | 388 | /* ... start it up. This will trade welcome banners, exchange keys, |
385 | 389 | * and setup crypto, compression, and MAC layers |
386 | 390 | */ |
| 391 | /* FIXME: Starting in libssh2 version 1.2.8 this function is considered deprecated. |
| 392 | Use libssh2_session_handshake instead. */ |
387 | 393 | rc = libssh2_session_startup (super_data->session, super_data->socket_handle); |
388 | 394 | if (rc != 0) |
389 | 395 | { |
… |
… |
sftpfs_close_connection (struct vfs_s_super *super, const char *shutdown_message |
430 | 436 | { |
431 | 437 | sftpfs_super_data_t *super_data; |
432 | 438 | |
433 | | mc_return_if_error (mcerror); |
| 439 | /* no mc_return_*_if_error() here because of abort open_connection handling too */ |
| 440 | (void) mcerror; |
434 | 441 | |
435 | 442 | super_data = (sftpfs_super_data_t *) super->data; |
436 | 443 | if (super_data == NULL) |
437 | 444 | return; |
438 | 445 | |
439 | | vfs_path_element_free (super_data->original_connection_info); |
440 | | super_data->original_connection_info = NULL; |
| 446 | if (super_data->sftp_session != NULL) |
| 447 | { |
| 448 | libssh2_sftp_shutdown (super_data->sftp_session); |
| 449 | super_data->sftp_session = NULL; |
| 450 | } |
441 | 451 | |
442 | 452 | if (super_data->agent != NULL) |
443 | 453 | { |
… |
… |
sftpfs_close_connection (struct vfs_s_super *super, const char *shutdown_message |
446 | 456 | super_data->agent = NULL; |
447 | 457 | } |
448 | 458 | |
449 | | if (super_data->sftp_session != NULL) |
450 | | { |
451 | | libssh2_sftp_shutdown (super_data->sftp_session); |
452 | | super_data->sftp_session = NULL; |
453 | | } |
| 459 | super_data->fingerprint = NULL; |
454 | 460 | |
455 | 461 | if (super_data->session != NULL) |
456 | 462 | { |
457 | 463 | libssh2_session_disconnect (super_data->session, shutdown_message); |
| 464 | libssh2_session_free(super_data->session); |
458 | 465 | super_data->session = NULL; |
459 | 466 | } |
460 | 467 | |
461 | | super_data->fingerprint = NULL; |
462 | | |
463 | 468 | if (super_data->socket_handle != -1) |
464 | 469 | { |
465 | 470 | close (super_data->socket_handle); |
diff --git a/src/vfs/sftpfs/vfs_subclass.c b/src/vfs/sftpfs/vfs_subclass.c
index 8ddff74..a5826e6 100644
a
|
b
|
static void |
137 | 137 | sftpfs_cb_close_connection (struct vfs_class *me, struct vfs_s_super *super) |
138 | 138 | { |
139 | 139 | GError *mcerror = NULL; |
| 140 | sftpfs_super_data_t *sftpfs_super_data; |
140 | 141 | |
141 | 142 | (void) me; |
142 | 143 | sftpfs_close_connection (super, "Normal Shutdown", &mcerror); |
143 | 144 | mc_error_message (&mcerror, NULL); |
| 145 | |
| 146 | sftpfs_super_data = (sftpfs_super_data_t *) super->data; |
| 147 | if (sftpfs_super_data != NULL) |
| 148 | vfs_path_element_free (sftpfs_super_data->original_connection_info); |
| 149 | |
144 | 150 | g_free (super->data); |
145 | 151 | } |
146 | 152 | |