#211: size mismatch between struct transportpacket fields causes libssh2 to get
stuck
---------------------------------------------------------------------------------------+
Reporter:
www.google.com/accounts/o8/id?id=aitoawlhggg_yplkl7grwwpbbum-omtqud4rmna |
Owner:
Type: defect
| Status: reopened
Priority: normal
| Milestone: 1.2.8
Component: protocol
| Version: 1.2.7
Resolution:
| Keywords:
Blocks:
| Blocked By:
---------------------------------------------------------------------------------------+
Changes (by
www.google.com/accounts/o8/id?id=aitoawlhggg_yplkl7grwwpbbum-omtqud4rmna):
* status: closed => reopened
* resolution: fixed =>
Comment:
I am reopening this bug as just using size_t does not resolve the problem.
libssh2 just gets stuck in the next read call on the session. Initially I
thought this was a problem with my code, since it was issuing reads even
after an error was returned. However on closer inspection, this read is
coming as a result of calling libssh_channel_free after the error. To
illustrate, here is the backtrace.
#0 0x0000000000a5b8db in _libssh2_transport_read (session=0x7f42e368ff80)
at transport.c:536
#1 0x0000000000a531e1 in _libssh2_channel_close (channel=0x7f42e3606a20)
at channel.c:2257
#2 0x0000000000a5339b in _libssh2_channel_free (channel=0x7f42e3606a20)
at channel.c:2386
#3 0x0000000000a533d8 in libssh2_channel_free (channel=0x7f42e3606a20) at
channel.c:2461
#4 0x00000000005b917a in SSHSession::clearChannel (this=0x7f42e29e11a0)
at sessions/ssh-session.cc:61
#5 0x00000000005b8247 in SSHSession::disconnect (this=0x7f42e29e11a0) at
sessions/ssh-session.cc:45
#6 0x00000000005beaf0 in QuerySession::errorOccurred
(this=0x7f42e29e11a0, sessErr=@0x1058640)
at sessions/query-session.cc:243
#7 0x00000000005bfc41 in QuerySession::doRead (this=0x7f42e29e11a0) at
sessions/query-session.cc:330
#8 0x00000000005b68f2 in SSHSession::doRead (this=0x7f42e29e11a0) at
sessions/ssh-session.cc:331
#9 0x00000000005c2091 in QuerySession::fdReadReady (this=0x7f42e29e11a0)
at sessions/query-session.cc:150
#10 0x00000000005b81ef in SSHSession::fdReadReady (this=0x7f42e29e11a0) at
sessions/ssh-session.cc:237
#11 0x00000000009f6481 in Scheduler::run ()
#12 0x000000000065f4b4 in main (argc=8, argv=0x7fffd9d24a18) at
main.cc:389
Current language: auto; currently c
i.e. the same bogus packet length value is causing libssh2 to get stuck in
the same way as described in comment #0.
Suggested solution - if returning an error (rc < 0) other than EAGAIN, set
session.packet.total_num = 0. An easy way to do this is to add a level on
indirection to _libssh2_transport_read for example by making
_libssh2_trasport_read be defined as
int _libssh2_tranport_read(LIBSSH2_SESSION * session){
int rc = libssh2_transport_read(session);
if (rc < 0 && rc != LIBSSH2_ERROR_EAGAIN)
session.packet.total_num = 0;
return rc;
}
where libssh2_trasport_read is the same as _libssh2_trasport_read is
current code.
For consistency of naming I could add a similar indirection in
_libssh2_transport_send. But since there is no point adding a extra
function call there we could define _libssh2_transport_send with a macro
(that just calls libssh2_transport_send).
Alternatively if it is important to not add the extra function call cost
to libssh2_transport_read. I can reset the session.packet.total_num value
inside the function in case of error. However this is more error prone.
Please reply and I can roll you a patch for this.
--
Ticket URL: <http://trac.libssh2.org/ticket/211#comment:2>
libssh2 <http://trac.libssh2.org/>
C library for writing portable SSH2 clients
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
