Subject: [libssh2] SCP upload issues

[libssh2] SCP upload issues

From: Erik Broßler <erik.brossler_at_web.de>
Date: Tue, 05 Feb 2008 16:54:27 +0100

Hello,

I am using libssh2 version 0.18 along with cURL 7.17.1 and 7.18.0 on several platforms (SuSE Linux 10, Solaris 10 x86, AIX 5.3).

When trying to upload large files via SCP using cURL, I repeatedly got the error message "curl: (55) Failed sending data to the peer". After some debugging, I found that this is caused by a bug in libssh2 - to be precise, in the function libssh2_channel_write_ex.

The issue can also be reproduced with the sample programs included in the libssh2 package. "scp", "scp_nonblock" and "scp_write" seem to work properly. However, "scp_write_nonblock" runs occasionally into a memory allocation error. With tracing enabled (LIBSSH2_TRACE_ERRORS | LIBSSH2_TRACE_CONN) I got the following output:
...
[libssh2] Connection: Writing 1024 bytes on channel 0/0, stream #0
[libssh2] Connection: Splitting write block due to 1000 byte window_size on 0/0/0
[libssh2] Connection: Sending 1000 bytes on channel 0/0, stream_id=0
[libssh2] Connection: Window adjust received for channel 0/0, adding 74776 bytes, new window_size=74776
[libssh2] Connection: Sending 1024 bytes on channel 0/0, stream_id=0
[libssh2] Connection: Writing -1000 bytes on channel 0/0, stream #0
[libssh2] Failure Event: -6 - Unable to allocte space for data transmission packet
ERROR -1
...
As you can see, a message had to be split due to window size limitations. In the first iteration, only 1000 out of 1024 bytes were transmitted. But in the next iteration, the function transmitted the full 1024 byte packet again, rather than only the 24 bytes left from the previous iteration. So the return value of libssh2_channel_write_ex() was 2024, though the calling program did only pass 1024 bytes - and this wrong return value messed up the caller's transfer byte counter. Due to this, the sample program passed a negative buffer size to the write function in the next iteraction, causing the memory allocation to fail.

With some more debugging code added to channel.c, I found, that this issue only occurs, if an idle loop (returning LIBSSH2_EAGAIN) occurs between two consecutive parts of a split message. If the whole message is transferred within a single invocation of libssh2_channel_write_ex, buflen is adjusted appropriately. But if the functions returns to the caller inbetween, the original buflen value is passed in again. That's why scp_write (blocking) worked properly, while scp_write_nonblock ran into the failure.

The reason for this is, that the value of channel->write_bufwrite is set to the full buffer length at channel.c[1695] rather than the difference between the buffer length and the already transferred byte count (buflen - channel->write_bufwrote). The patch appended blow will fix this issue and adjusts some other variables accordingly.

With this patch applied, arbitrary test files could be uploaded properly with both, the sample programs and cURL.

Cheers,
Erik Brossler

diff -Naur libssh2-0.18-orig/src/channel.c libssh2-0.18/src/channel.c
--- libssh2-0.18-orig/src/channel.c 2007-11-08 16:11:33.000000000 +0100
+++ libssh2-0.18/src/channel.c 2008-02-04 20:48:30.000000000 +0100
@@ -1690,9 +1690,9 @@
         channel->write_state = libssh2_NB_state_allocated;
     }
 
- while (buflen > 0) {
+ while (buflen > channel->write_bufwrote) {
         if (channel->write_state == libssh2_NB_state_allocated) {
- channel->write_bufwrite = buflen;
+ channel->write_bufwrite = buflen - channel->write_bufwrote;
             channel->write_s = channel->write_packet;
 
             *(channel->write_s++) =
@@ -1714,6 +1714,7 @@
                 if (rc < 0) {
                     /* Error or EAGAIN occurred, disconnect? */
                     if (rc != PACKET_EAGAIN) {
+ LIBSSH2_FREE(session, channel->write_packet);
                         channel->write_state = libssh2_NB_state_idle;
                     }
                     return rc;
@@ -1747,7 +1748,7 @@
             }
             libssh2_htonu32(channel->write_s, channel->write_bufwrite);
             channel->write_s += 4;
- memcpy(channel->write_s, buf, channel->write_bufwrite);
+ memcpy(channel->write_s, buf + channel->write_bufwrote, channel->write_bufwrite);
             channel->write_s += channel->write_bufwrite;
 
             _libssh2_debug(session, LIBSSH2_DBG_CONN,
@@ -1774,10 +1775,6 @@
             }
             /* Shrink local window size */
             channel->local.window_size -= channel->write_bufwrite;
-
- /* Adjust buf for next iteration */
- buflen -= channel->write_bufwrite;
- buf += channel->write_bufwrite;
             channel->write_bufwrote += channel->write_bufwrite;
 
             channel->write_state = libssh2_NB_state_allocated;

_____________________________________________________________________
Der WEB.DE SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen!
http://smartsurfer.web.de/?mc=100071&distributionid=000000000066

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
libssh2-devel mailing list
libssh2-devel_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2008-02-05