Subject: Re: [libssh2] #211: size mismatch between struct transportpacket fields causes libssh2 to get stuck

Re: [libssh2] #211: size mismatch between struct transportpacket fields causes libssh2 to get stuck

From: libssh2 Trac <trac_at_libssh2.stuge.se>
Date: Sat, 26 Feb 2011 00:57:27 -0000

#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
Received on 2011-02-26