Bug 178055 - tftp / tftpd file transfer ends up with incorrect size
Summary: tftp / tftpd file transfer ends up with incorrect size
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Alan Somers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-22 18:20 UTC by Richard
Modified: 2018-10-01 16:13 UTC (History)
1 user (show)

See Also:
asomers: mfc-stable11+
asomers: mfc-stable10+


Attachments
A reproduction case (9.82 KB, application/x-bzip)
2018-08-22 22:23 UTC, Alan Somers
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard 2013-04-22 18:20:00 UTC
On transferring large files tftp / tftpd combo fails to transfer correct file;
file is truncated on my checks.

Use of '/dev/zero' to populate a "bigfile" created a situation that did not exhibit the problem.  Use of '/dev/random' to populate a "bigfile" led to situation that did exhibit problem on 2 out of 2 attempts.

When I replaced "/usr/libexec/tftpd" with the ports tftp-hpa "/usr/local/libexec/in.tftpd" did resolve the file transfer issue.

I'll note that using the "hpa" version led to some confusion with the tftp client: I was forced to disable "options" to allow the tftp client to work - which seemed a bit off to me.

How-To-Repeat: setup server side:
> su
* enable tftpd in /etc/inetd.conf
> cd /tftpboot
> dd if=/dev/random of=bigfile bs=21504 count=10000

on same system:
> cd /tmp
> tftp localhost
> get bigfile
> ^D
> ls -l bigfile
Comment 1 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:00:15 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 2 Alan Somers freebsd_committer freebsd_triage 2018-08-22 16:03:08 UTC
Reproducible on FreeBSD 12.
Comment 3 Alan Somers freebsd_committer freebsd_triage 2018-08-22 22:22:34 UTC
The problem isn't the file's size; it's got to do with the netascii encoding.  If a transfer block ends with CR, tftpd erroneously omits the leading NUL from the following transfer block.  I have a fix for that.  However, what I don't understand is why the bug doesn't occur more often.  Sometimes tftp is able to correctly decode the incorrectly-encoded packets, and sometimes it isn't.  I don't know why but I've already spent too much time trying to figure it out, so I'm just going to fix the bug in the server instead.
Comment 4 Alan Somers freebsd_committer freebsd_triage 2018-08-22 22:23:35 UTC
Created attachment 196452 [details]
A reproduction case

This is the smallest file I could craft that triggers the bug.  extract it, then transfer it in netascii mode.  One NUL byte will be dropped at about the halfway point.
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-08-22 23:31:31 UTC
A commit references this bug:

Author: asomers
Date: Wed Aug 22 23:31:28 UTC 2018
New revision: 338216
URL: https://svnweb.freebsd.org/changeset/base/338216

Log:
  tftpd: Fix data corruption bug with netascii

  Transferring files in netascii format requires, among other things,
  translating all CR characters to a CR,NUL pair. tftpd does this correctly
  except when the CR occurs as the last octet of a packet. In that case, it
  erroneously drops the NUL which should be part of the following packet. The
  bug was caused by using 0 as a sentinel value in a variable that could
  legitimately hold 0. Fix it by switching the sentinel value to -1.

  PR:		178055
  Reported by:	Richard <rsitze@gmail.com>
  Reviewed by:	cem
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D16853

Changes:
  head/libexec/tftpd/tftp-file.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2018-10-01 16:04:28 UTC
A commit references this bug:

Author: asomers
Date: Mon Oct  1 16:04:07 UTC 2018
New revision: 339057
URL: https://svnweb.freebsd.org/changeset/base/339057

Log:
  MFC r338216:

  tftpd: Fix data corruption bug with netascii

  Transferring files in netascii format requires, among other things,
  translating all CR characters to a CR,NUL pair. tftpd does this correctly
  except when the CR occurs as the last octet of a packet. In that case, it
  erroneously drops the NUL which should be part of the following packet. The
  bug was caused by using 0 as a sentinel value in a variable that could
  legitimately hold 0. Fix it by switching the sentinel value to -1.

  PR:		178055
  Reported by:	Richard <rsitze@gmail.com>
  Reviewed by:	cem
  Differential Revision:	https://reviews.freebsd.org/D16853

Changes:
_U  stable/11/
  stable/11/libexec/tftpd/tftp-file.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2018-10-01 16:11:35 UTC
A commit references this bug:

Author: asomers
Date: Mon Oct  1 16:11:09 UTC 2018
New revision: 339062
URL: https://svnweb.freebsd.org/changeset/base/339062

Log:
  MFC r338216:

  tftpd: Fix data corruption bug with netascii

  Transferring files in netascii format requires, among other things,
  translating all CR characters to a CR,NUL pair. tftpd does this correctly
  except when the CR occurs as the last octet of a packet. In that case, it
  erroneously drops the NUL which should be part of the following packet. The
  bug was caused by using 0 as a sentinel value in a variable that could
  legitimately hold 0. Fix it by switching the sentinel value to -1.

  PR:		178055
  Reported by:	Richard <rsitze@gmail.com>
  Reviewed by:	cem
  Differential Revision:	https://reviews.freebsd.org/D16853

Changes:
_U  stable/10/
  stable/10/libexec/tftpd/tftp-file.c