Bugs item #3004787, was opened at 2010-05-20 18:07
Message generated for change (Settings changed) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3004787&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: TFTP
Group: wrong behaviour
Status: Open
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Douglas Kilpatrick (kilpatds)
Assigned to: Daniel Stenberg (bagder)
Summary: Curl's tftp does not properly handle block number wrap.

Initial Comment:
In the tftp protocol, the block count is a 16-bit unsigned variable, that starts at 1 Obviously, this will wrap after around 65k blocks. In a non-multicast situation, that's usually not a problem as the protocol only allows one outstanding packet at any given point in time. So ... block# 65535 is followed by block# 0.

However, curl doesn't seem to support this correctly. The tftp binary is the simple (doesn't support options) system binary, also using a 512 block size.

$ ls -l /tftpboot/foo4
-rw-r--r-- 1 root root 33554944 May 17 14:53 /tftpboot/foo4
$ rm -f foo4 && tftp localhost -c get foo4 && ls -l foo4
-rw-rw-r-- 1 dkilpatr dkilpatr 33554944 May 20 11:59 foo4
$ rm -f foo4 && ~/bin/curl -s -o foo4 tftp://localhost/foo4 && ls -l foo4
-rw-rw-r-- 1 dkilpatr dkilpatr 33554432 May 20 11:59 foo4
$ ~/bin/curl -V
curl 7.20.1 (x86_64-unknown-linux-gnu) libcurl/7.20.1 OpenSSL/0.9.8b zlib/1.2.3 libidn/0.6.5
Protocols: dict file ftp ftps http https imap imaps ldap pop3 pop3s rtsp smtp smtps telnet tftp
Features: IDN IPv6 Largefile NTLM SSL libz
$ echo $(( 33554944 / 512 ))
65537
$ echo $(( 33554432 / 512 ))
65536

That 65536 blocks were saved is somewhat surprising... the block number starts at 1, and so only 65535 blocks are transfered before the blocknumber wraps.

----------------------------------------------------------------------

>Comment By: Daniel Stenberg (bagder)
Date: 2010-05-21 23:11

Message:
Thanks a lot. I did the fix slightly different but your suggested certainly
inspired me.

I can only agree to your follow-up comment as well and I committed a
change for that as well.

I think this bug report is fixed now! Please let me know if you still
experience problems in the git version or starting with tomorrow's daily
snapshot.

----------------------------------------------------------------------

Comment By: Douglas Kilpatrick (kilpatds)
Date: 2010-05-21 19:28

Message:
On a different, but related note...

shouldn't there be a return on line 600 of tftp.c? You're falling out of
the "block numbers don't match" code through a comment that says "this is
the expected block"

----------------------------------------------------------------------

Comment By: Douglas Kilpatrick (kilpatds)
Date: 2010-05-21 04:36

Message:
The basic issue is "unsigned short + int" is not an unsigned short, and
thus wraps differently.

(gdb) print (((unsigned short)65535)+1)
$1 = 65536
(gdb) print ((unsigned short)(65535+1))
$3 = 0

The applied patch is one solution.

$ ls -l /var/lib/tftpboot/foo
-rw-r--r--. 1 root root 33554944 2010-05-20 21:46 /var/lib/tftpboot/foo
$ ./curl --trace foo.trace -o foo tftp://74.94.212.155/foo
  % Total % Received % Xferd Average Speed Time Time Time
Current
                                 Dload Upload Total Spent Left
Speed
 99 32.0M 99 32.0M 0 0 10.4M 0 0:00:03 0:00:03 --:--:--
10.4M
$ tail -8 foo.trace
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: Received unexpected DATA packet block 0
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: DO phase is complete
== Info: Closing connection #0
$ (cd ..; patch -p0 < tftp.patch; make > /dev/null 2>&1 )
patching file lib/tftp.c
$ ./curl --trace foo.trace -o foo tftp://74.94.212.155/foo
  % Total % Received % Xferd Average Speed Time Time Time
Current
                                 Dload Upload Total Spent Left
Speed
100 32.0M 100 32.0M 0 0 10.8M 0 0:00:02 0:00:02 --:--:--
10.8M
$ tail -8 foo.trace
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: TFTP_STATE_RX
== Info: DO phase is complete
== Info: Closing connection #0
$ ls -l /var/lib/tftpboot/foo
-rw-r--r--. 1 root root 33554944 2010-05-20 21:46 /var/lib/tftpboot/foo

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2010-05-20 19:58

Message:
Can you help us track down the exact problem? The counter is clearly an
unsigned short and it will wrap, but obviously something in the wrap
procedure or so isn't functioning as it should...

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3004787&group_id=976
Received on 2010-05-22