Subject: The little SFTP flaw I found

The little SFTP flaw I found

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Tue, 14 Dec 2010 14:30:24 +0100 (CET)

hi

(I decided to push my code and not do any fix for this right now, and here I
try to describe the little problem.)

Currently I've made the 'packet_list' a linked list of packets that is kept
within the SFTP handle that you get returned when you call sftp_open().

The list keeps track of all outstanding "pipelined" requests and using that
list the code maps incoming ACKs etc against that list. The incoming SFTP
packets are kept in a list that is "global" for the SFTP session, as an SFTP
session is using a single SSH channel during its entire life span which may do
many different SFTP operations.

The problem with this is when we have outstanding SFTP packets at the time we
close an SFTP handle, as then we nuke the list of known outstanding packets
and when the server responds the packets are added to the queue but will never
be read off from it. "outstanding packets" are for example read-head reads
that is sent off to read before we knew that the file actually has ended.
(Each outgoing packet has a 32bit 'id' field and each incoming packet has one
too, and that's what maps requests with responses.)

Since the ids are sequential and used for the SFTP session and not on a
per-handle basis I figure moving the list to the handle is a good idea anyway.
Then we need to add a way to make sure each handle knows which outgoing
packets that it added there. I figure we can start with simply storing a
within each struct back to the correct SFTP handle.

When a single handle is removed, we go through the list of outstanding request
that haven't yet received a response and we "unmark" the request as having a
handle as then we can discard received response packets to such requests
immediately on sight.

However, with a better STAT done on the file before SFTP downloads we should
do MUCH less over-reads and thus the problem will be reduced drastically, so I
think I'll prioritize the STAT fix.

Unless someone else have any other great ideas, thoughts or comments that make
change my mind! ;-)

-- 
  / daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2010-12-14