Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[PATCH] sftp: fix connection memleaks #3581

Closed
mc-butler opened this issue Jan 3, 2016 · 18 comments
Closed

[PATCH] sftp: fix connection memleaks #3581

mc-butler opened this issue Jan 3, 2016 · 18 comments
Assignees
@aborodin
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress
Milestone
4.8.16

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/3581
Reporter and

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

Note

Original attachments:
@mc-butler
Copy link
Author

Changed by and on Jan 3, 2016 at 15:57 UTC

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 3, 2016 at 16:18 UTC (comment 1)

Is it possible to split this patch into several ones with independent issues?

@mc-butler
Copy link
Author

Changed by and on Jan 3, 2016 at 17:15 UTC (comment 2)

sure, I will do so.

@mc-butler
Copy link
Author

Changed by and on Jan 3, 2016 at 18:00 UTC

@mc-butler
Copy link
Author

Changed by and on Jan 3, 2016 at 18:00 UTC

@mc-butler
Copy link
Author

Changed by and on Jan 3, 2016 at 18:00 UTC

@mc-butler
Copy link
Author

Changed by and on Jan 3, 2016 at 18:01 UTC

@mc-butler
Copy link
Author

Changed by and on Jan 3, 2016 at 18:01 UTC

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 7, 2016 at 8:18 UTC (comment 3)

  • Status changed from new to accepted
  • Component changed from mc-core to mc-vfs
  • Owner set to andrew_b
  • Branch state changed from no branch to on review

Thanks!

Branch: 3581_sftp_fixes
Initial [fa4056b405a47935aa9fd21370d0d0fe92b41d7a]

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Jan 10, 2016 at 8:33 UTC (comment 4)

Hi Andreas,

Maybe you will find it interesting to have a look at this Debian bug, which also has to do with SFTP:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=774135

So, in essence, the user is running Debian x32 (!) and there SFTP is completely broken. Probably this happens due to some type size weirdness on x32, because we can't reproduce it on i386 and amd64.

--Yury

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 10, 2016 at 13:50 UTC (comment 4.5)

Replying to zaytsev:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=774135

So, in essence, the user is running Debian x32 (!) and there SFTP is completely broken. Probably this happens due to some type size weirdness on x32, because we can't reproduce it on i386 and amd64.

I've added fix of config file reading. I don't think that it fixes that bug, but who knows.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Jan 10, 2016 at 15:12 UTC (comment 6)

Some new info from the Debian bug, with 4.8.15 it looks like this:

$ touch .ssh/config
$ mc
$ cd /sftp://localhost

╔═════════════════════════════════════════ Error ═════════════════════════════════════════╗
║                                                                                         ║
║ 25: sftp: an error occurred while reading ~/.ssh/config: Inappropriate ioctl for device ║
║                                                                                         ║
╚═════════════════════════════════════════════════════════════════════════════════════════╝

╔═══════════════ Error ════════════════╗
║                                      ║
║ Cannot chdir to "/sftp://localhost/" ║
║           File exists (17)           ║
║                                      ║
╚══════════════════════════════════════╝

Asked to test with the latest 3581_sftp_fixes branch...

@mc-butler
Copy link
Author

Changed by and on Jan 11, 2016 at 21:09 UTC (comment 7)

Can Thorsten test following patch?

when reading ssh_config with matching host entry mc do bad pointer arithmetic

@mc-butler
Copy link
Author

Changed by and on Jan 11, 2016 at 21:11 UTC

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jan 26, 2016 at 16:58 UTC (comment 8)

Ticket #3590 has been marked as a duplicate of this ticket.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 13, 2016 at 13:43 UTC (comment 9)

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

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 13, 2016 at 13:44 UTC (comment 10)

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

Merged to master: [ab0c2af].

git log --pretty=oneline 9dcf033..ab0c2af

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 13, 2016 at 13:45 UTC (comment 11)

  • Status changed from testing to closed

@mc-butler mc-butler marked this as a duplicate of #3590 Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aborodin aborodin

Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress
Milestone
4.8.16
Development

No branches or pull requests
2 participants
@aborodin @mc-butler