Fixing several timeout quirks at tftpd and tftp:

- move TIMEOUT* defines to arpa/tftp.h, as they are used several times
  in tftpd and tftp, and the values are part of the RFC definition.
- tftpd and tftp did count the total retransmission time in retries
  instead in seconds.  fixed.
- tftpd rexmt timeout was hardcoded by a define and therefore didn't
  changed when the timeout option was sent.  fixed.
- limit total retransmission timeout in tftp to also 255 seconds.
- replace obvious atoi()'s by strtonum().

ok claudio@

5 files changed, 41 insertions, 38 deletions

diff --git a/include/arpa/tftp.h b/include/arpa/tftp.h
index 0f6fe83ae25..8f9d4b89dc1 100644
--- a/include/arpa/tftp.h
+++ b/include/arpa/tftp.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tftp.h,v 1.6 2006/07/20 09:42:44 mglocker Exp $	*/
+/*	$OpenBSD: tftp.h,v 1.7 2006/07/26 09:10:03 mglocker Exp $	*/
 /*	$NetBSD: tftp.h,v 1.3 1994/10/26 00:56:48 cgd Exp $	*/
 
 /*
@@ -41,6 +41,9 @@
 #define	SEGSIZE		512		/* data segment size */
 #define SEGSIZE_MIN	8		/* minimal data segment size */
 #define SEGSIZE_MAX	65464		/* maximal data segment size */
+#define TIMEOUT		5		/* packet rexmt timeout */
+#define TIMEOUT_MIN	1		/* minimal packet rexmt timeout */
+#define TIMEOUT_MAX	255		/* maximal packet rexmt timeout */
 
 /*
  * Packet types.
diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c
index e323922bbca..5a2fa283947 100644
--- a/libexec/tftpd/tftpd.c
+++ b/libexec/tftpd/tftpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tftpd.c,v 1.48 2006/07/21 21:28:47 mglocker Exp $	*/
+/*	$OpenBSD: tftpd.c,v 1.49 2006/07/26 09:10:03 mglocker Exp $	*/
 
 /*
  * Copyright (c) 1983 Regents of the University of California.
@@ -37,7 +37,7 @@ char copyright[] =
 
 #ifndef lint
 /*static char sccsid[] = "from: @(#)tftpd.c	5.13 (Berkeley) 2/26/91";*/
-static char rcsid[] = "$OpenBSD: tftpd.c,v 1.48 2006/07/21 21:28:47 mglocker Exp $";
+static char rcsid[] = "$OpenBSD: tftpd.c,v 1.49 2006/07/26 09:10:03 mglocker Exp $";
 #endif /* not lint */
 
 /*
@@ -68,9 +68,6 @@ static char rcsid[] = "$OpenBSD: tftpd.c,v 1.48 2006/07/21 21:28:47 mglocker Exp
 #include <unistd.h>
 #include <vis.h>
 
-#define	TIMEOUT		5
-#define	MAX_TIMEOUTS	5
-
 struct formats;
 
 int		readit(FILE *, struct tftphdr **, int, int);
@@ -92,7 +89,7 @@ extern char		 *__progname;
 struct sockaddr_storage	  s_in;
 int			  peer;
 int			  rexmtval = TIMEOUT;
-int			  max_rexmtval = 2 * TIMEOUT;
+int			  maxtimeout = 5 * TIMEOUT;
 char			 *buf;
 char			 *ackbuf;
 struct sockaddr_storage	  from;
@@ -452,14 +449,13 @@ again:
 
 option_fail:
 	if (options[OPT_TIMEOUT].o_request) {
-		int to = atoi(options[OPT_TIMEOUT].o_request);
-		if (to < 1 || to > 255) {
+		int to = strtonum(options[OPT_TIMEOUT].o_request,
+		    TIMEOUT_MIN, TIMEOUT_MAX, &errstr);
+		if (errstr) {
 			nak(EBADOP);
 			exit(1);
-		} else if (to <= max_rexmtval)
-			options[OPT_TIMEOUT].o_reply = rexmtval = to;
-		else
-			options[OPT_TIMEOUT].o_request = NULL;
+		}
+		options[OPT_TIMEOUT].o_reply = rexmtval = to;
 	}
 
 	if (options[OPT_BLKSIZE].o_request) {
@@ -611,7 +607,7 @@ sendfile(struct formats *pf)
 
 		/* send data to client and wait for client ACK */
 		for (timeouts = 0, error = 0;;) {
-			if (timeouts == MAX_TIMEOUTS)
+			if (timeouts >= maxtimeout)
 				exit(1);
 
 			if (!error) {
@@ -625,9 +621,9 @@ sendfile(struct formats *pf)
 
 			pfd[0].fd = peer;
 			pfd[0].events = POLLIN;
-			nfds = poll(pfd, 1, TIMEOUT * 1000);
+			nfds = poll(pfd, 1, rexmtval * 1000);
 			if (nfds == 0) {
-				timeouts++;
+				timeouts += rexmtval;
 				continue;
 			}
 			if (nfds == -1) {
@@ -698,7 +694,7 @@ recvfile(struct formats *pf)
 
 		/* send ACK to client and wait for client data */
 		for (timeouts = 0, error = 0;;) {
-			if (timeouts == MAX_TIMEOUTS)
+			if (timeouts >= maxtimeout)
 				exit(1);
 
 			if (!error) {
@@ -712,9 +708,9 @@ recvfile(struct formats *pf)
 
 			pfd[0].fd = peer;
 			pfd[0].events = POLLIN;
-			nfds = poll(pfd, 1, TIMEOUT * 1000);
+			nfds = poll(pfd, 1, rexmtval * 1000);
 			if (nfds == 0) {
-				timeouts++;
+				timeouts += rexmtval;
 				continue;
 			}
 			if (nfds == -1) {
@@ -772,7 +768,7 @@ noack:
 	/* just quit on timeout */
 	pfd[0].fd = peer;
 	pfd[0].events = POLLIN;
-	nfds = poll(pfd, 1, TIMEOUT * 1000);
+	nfds = poll(pfd, 1, rexmtval * 1000);
 	if (nfds < 1)
 		exit(1);
 	n = recv(peer, buf, packet_size, 0);
@@ -853,7 +849,7 @@ oack(int opcode)
 
 	/* send OACK to client and wait for client ACK */
 	for (timeouts = 0, error = 0;;) {
-		if (timeouts == MAX_TIMEOUTS)
+		if (timeouts >= maxtimeout)
 			exit(1);
 
 		if (!error) {
@@ -866,9 +862,9 @@ oack(int opcode)
 
 		pfd[0].fd = peer;
 		pfd[0].events = POLLIN;
-		nfds = poll(pfd, 1, TIMEOUT * 1000);
+		nfds = poll(pfd, 1, rexmtval * 1000);
 		if (nfds == 0) {
-			timeouts++;
+			timeouts += rexmtval;
 			continue;
 		}
 		if (nfds == -1) {
diff --git a/usr.bin/tftp/main.c b/usr.bin/tftp/main.c
index ce9d6ecb793..006a6601ecb 100644
--- a/usr.bin/tftp/main.c
+++ b/usr.bin/tftp/main.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: main.c,v 1.26 2006/07/24 17:29:58 mglocker Exp $	*/
+/*	$OpenBSD: main.c,v 1.27 2006/07/26 09:10:03 mglocker Exp $	*/
 /*	$NetBSD: main.c,v 1.6 1995/05/21 16:54:10 mycroft Exp $	*/
 
 /*
@@ -41,7 +41,7 @@ static const char copyright[] =
 static char sccsid[] = "@(#)main.c	8.1 (Berkeley) 6/6/93";
 #endif
 static const char rcsid[] =
-    "$OpenBSD: main.c,v 1.26 2006/07/24 17:29:58 mglocker Exp $";
+    "$OpenBSD: main.c,v 1.27 2006/07/26 09:10:03 mglocker Exp $";
 #endif /* not lint */
 
 /*
@@ -71,7 +71,6 @@ static const char rcsid[] =
 
 #include "extern.h"
 
-#define	TIMEOUT		5		/* secs between rexmt's */
 #define	LBUFLEN		200		/* size of input buffer */
 #define	MAXARGV		20
 #define HELPINDENT	(sizeof("connect"))
@@ -519,7 +518,7 @@ setrexmt(int argc, char *argv[])
 		printf("usage: %s value\n", argv[0]);
 		return;
 	}
-	t = strtonum(argv[1], 1, 255, &errstr);
+	t = strtonum(argv[1], TIMEOUT_MIN, TIMEOUT_MAX, &errstr);
 	if (errstr)
 		printf("%s: value is %s\n", argv[1], errstr);
 	else
@@ -529,7 +528,8 @@ setrexmt(int argc, char *argv[])
 void
 settimeout(int argc, char *argv[])
 {
-	int	t;
+	int		 t;
+	const char	*errstr;
 
 	if (argc < 2) {
 		strlcpy(line, "Maximum-timeout ", sizeof(line));
@@ -545,8 +545,9 @@ settimeout(int argc, char *argv[])
 		return;
 	}
 	t = atoi(argv[1]);
-	if (t < 0)
-		printf("%s: bad value\n", argv[1]);
+	t = strtonum(argv[1], TIMEOUT_MIN, TIMEOUT_MAX, &errstr);
+	if (errstr)
+		printf("%s: value is %s\n", argv[1], errstr);
 	else
 		maxtimeout = t;
 }
diff --git a/usr.bin/tftp/tftp.1 b/usr.bin/tftp/tftp.1
index 3f2ee236281..52105f9652f 100644
--- a/usr.bin/tftp/tftp.1
+++ b/usr.bin/tftp/tftp.1
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: tftp.1,v 1.14 2006/07/24 21:29:55 jmc Exp $
+.\"	$OpenBSD: tftp.1,v 1.15 2006/07/26 09:10:03 mglocker Exp $
 .\"	$NetBSD: tftp.1,v 1.5 1995/08/18 14:45:44 pk Exp $
 .\"
 .\" Copyright (c) 1990, 1993, 1994
@@ -189,6 +189,8 @@ Show current status.
 .Pp
 .It Ic timeout Ar total-transmission-timeout
 Set the total transmission timeout, in seconds.
+The default value is 25 seconds.
+Valid values are 1 second \(en 255 seconds.
 .Pp
 .It Ic tout
 Toggle the
diff --git a/usr.bin/tftp/tftp.c b/usr.bin/tftp/tftp.c
index 9c817990407..3638b2731fe 100644
--- a/usr.bin/tftp/tftp.c
+++ b/usr.bin/tftp/tftp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tftp.c,v 1.19 2006/07/24 17:29:58 mglocker Exp $	*/
+/*	$OpenBSD: tftp.c,v 1.20 2006/07/26 09:10:03 mglocker Exp $	*/
 /*	$NetBSD: tftp.c,v 1.5 1995/04/29 05:55:25 cgd Exp $	*/
 
 /*
@@ -35,7 +35,7 @@
 static char sccsid[] = "@(#)tftp.c	8.1 (Berkeley) 6/6/93";
 #endif
 static const char rcsid[] =
-    "$OpenBSD: tftp.c,v 1.19 2006/07/24 17:29:58 mglocker Exp $";
+    "$OpenBSD: tftp.c,v 1.20 2006/07/26 09:10:03 mglocker Exp $";
 #endif /* not lint */
 
 /*
@@ -162,7 +162,7 @@ sendfile(int fd, char *name, char *mode)
 
 		/* send data to server and wait for server ACK */
 		for (timeouts = 0, error = 0; !intrflag;) {
-			if (timeouts == maxtimeout) {
+			if (timeouts >= maxtimeout) {
 				printtimeout();
 				goto abort;
 			}
@@ -185,7 +185,7 @@ sendfile(int fd, char *name, char *mode)
 			pfd[0].events = POLLIN;
 			nfds = poll(pfd, 1, rexmtval * 1000);
 			if (nfds == 0) {
-				timeouts++;
+				timeouts += rexmtval;
 				continue;
 			}
 			if (nfds == -1) {
@@ -295,7 +295,7 @@ options:
 
 		/* send ACK to server and wait for server data */
 		for (timeouts = 0, error = 0; !intrflag;) {
-			if (timeouts == maxtimeout) {
+			if (timeouts >= maxtimeout) {
 				printtimeout();
 				goto abort;
 			}
@@ -317,7 +317,7 @@ options:
 			pfd[0].events = POLLIN;
 			nfds = poll(pfd, 1, rexmtval * 1000);
 			if (nfds == 0) {
-				timeouts++;
+				timeouts += rexmtval;
 				continue;
 			}
 			if (nfds == -1) {
@@ -602,7 +602,8 @@ oack_set(const char *option, const char *value)
 			}
 			if (i == OPT_TIMEOUT) {
 				/* verify OACK response */
-				n = strtonum(value, 1, 255, &errstr);
+				n = strtonum(value, TIMEOUT_MIN, TIMEOUT_MAX,
+				    &errstr);
 				if (errstr || rexmtval != n ||
 				    opt_tout == 0) {
 					nak(EOPTNEG);