Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 29 Nov 2018 12:12:42 +0000 (UTC)
From:      Toomas Soome <tsoome@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r341224 - stable/12/stand/libsa
Message-ID:  <201811291212.wATCCgxc095327@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: tsoome
Date: Thu Nov 29 12:12:42 2018
New Revision: 341224
URL: https://svnweb.freebsd.org/changeset/base/341224

Log:
  MFC r339651,339992-339993,340026
  
  libsa: re-send ACK for older data packets in tftp
  libsa: tftp should not read past file end
  libsa: tftp should use calloc
  libsa: cstyle cleanup tftp.c
  
  Make loader pxeboot more stable over tftp transport.

Modified:
  stable/12/stand/libsa/tftp.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/stand/libsa/tftp.c
==============================================================================
--- stable/12/stand/libsa/tftp.c	Thu Nov 29 09:54:27 2018	(r341223)
+++ stable/12/stand/libsa/tftp.c	Thu Nov 29 12:12:42 2018	(r341224)
@@ -63,30 +63,29 @@ __FBSDID("$FreeBSD$");
 struct tftp_handle;
 struct tftprecv_extra;
 
-static ssize_t recvtftp(struct iodesc *d, void **pkt, void **payload,
-    time_t tleft, void *recv_extra);
-static int	tftp_open(const char *path, struct open_file *f);
-static int	tftp_close(struct open_file *f);
-static int	tftp_parse_oack(struct tftp_handle *h, char *buf, size_t len);
-static int	tftp_read(struct open_file *f, void *buf, size_t size, size_t *resid);
-static off_t	tftp_seek(struct open_file *f, off_t offset, int where);
-static int	tftp_set_blksize(struct tftp_handle *h, const char *str);
-static int	tftp_stat(struct open_file *f, struct stat *sb);
+static ssize_t recvtftp(struct iodesc *, void **, void **, time_t, void *);
+static int tftp_open(const char *, struct open_file *);
+static int tftp_close(struct open_file *);
+static int tftp_parse_oack(struct tftp_handle *, char *, size_t);
+static int tftp_read(struct open_file *, void *, size_t, size_t *);
+static off_t tftp_seek(struct open_file *, off_t, int);
+static int tftp_set_blksize(struct tftp_handle *, const char *);
+static int tftp_stat(struct open_file *, struct stat *);
 
 struct fs_ops tftp_fsops = {
-	"tftp",
-	tftp_open,
-	tftp_close,
-	tftp_read,
-	null_write,
-	tftp_seek,
-	tftp_stat,
-	null_readdir
+	.fs_name = "tftp",
+	.fo_open = tftp_open,
+	.fo_close = tftp_close,
+	.fo_read = tftp_read,
+	.fo_write = null_write,
+	.fo_seek = tftp_seek,
+	.fo_stat = tftp_stat,
+	.fo_readdir = null_readdir
 };
 
 extern struct in_addr servip;
 
-static int      tftpport = 2000;
+static int	tftpport = 2000;
 static int	is_open = 0;
 
 /*
@@ -94,21 +93,21 @@ static int	is_open = 0;
  * TFTP_REQUESTED_BLKSIZE of 1428 is (Ethernet MTU, less the TFTP, UDP and
  * IP header lengths).
  */
-#define TFTP_REQUESTED_BLKSIZE 1428
+#define	TFTP_REQUESTED_BLKSIZE 1428
 
 /*
  * Choose a blksize big enough so we can test with Ethernet
  * Jumbo frames in the future.
  */
-#define TFTP_MAX_BLKSIZE 9008
+#define	TFTP_MAX_BLKSIZE 9008
 
 struct tftp_handle {
 	struct iodesc  *iodesc;
-	int             currblock;	/* contents of lastdata */
-	int             islastblock;	/* flag */
-	int             validsize;
-	int             off;
-	char           *path;	/* saved for re-requests */
+	int		currblock;	/* contents of lastdata */
+	int		islastblock;	/* flag */
+	int		validsize;
+	int		off;
+	char		*path;	/* saved for re-requests */
 	unsigned int	tftp_blksize;
 	unsigned long	tftp_tsize;
 	void		*pkt;
@@ -117,7 +116,7 @@ struct tftp_handle {
 
 struct tftprecv_extra {
 	struct tftp_handle	*tftp_handle;
-	unsigned short		 rtype;		/* Received type */
+	unsigned short		rtype;		/* Received type */
 };
 
 #define	TFTP_MAX_ERRCODE EOPTNEG
@@ -141,42 +140,42 @@ tftp_senderr(struct tftp_handle *h, u_short errcode, c
 {
 	struct {
 		u_char header[HEADER_SIZE];
-		struct tftphdr  t;
+		struct tftphdr t;
 		u_char space[63]; /* +1 from t */
 	} __packed __aligned(4) wbuf;
-	char           *wtail;
-	int             len;
+	char *wtail;
+	int len;
 
 	len = strlen(msg);
 	if (len > sizeof(wbuf.space))
 		len = sizeof(wbuf.space);
 
-	wbuf.t.th_opcode = htons((u_short) ERROR);
-	wbuf.t.th_code   = htons(errcode);
+	wbuf.t.th_opcode = htons((u_short)ERROR);
+	wbuf.t.th_code = htons(errcode);
 
 	wtail = wbuf.t.th_msg;
 	bcopy(msg, wtail, len);
 	wtail[len] = '\0';
 	wtail += len + 1;
 
-	sendudp(h->iodesc, &wbuf.t, wtail - (char *) &wbuf.t);
+	sendudp(h->iodesc, &wbuf.t, wtail - (char *)&wbuf.t);
 }
 
 static void
-tftp_sendack(struct tftp_handle *h)
+tftp_sendack(struct tftp_handle *h, u_short block)
 {
 	struct {
 		u_char header[HEADER_SIZE];
 		struct tftphdr  t;
 	} __packed __aligned(4) wbuf;
-	char           *wtail;
+	char *wtail;
 
-	wbuf.t.th_opcode = htons((u_short) ACK);
-	wtail = (char *) &wbuf.t.th_block;
-	wbuf.t.th_block = htons((u_short) h->currblock);
+	wbuf.t.th_opcode = htons((u_short)ACK);
+	wtail = (char *)&wbuf.t.th_block;
+	wbuf.t.th_block = htons(block);
 	wtail += 2;
 
-	sendudp(h->iodesc, &wbuf.t, wtail - (char *) &wbuf.t);
+	sendudp(h->iodesc, &wbuf.t, wtail - (char *)&wbuf.t);
 }
 
 static ssize_t
@@ -190,7 +189,7 @@ recvtftp(struct iodesc *d, void **pkt, void **payload,
 	ssize_t len;
 
 	errno = 0;
-	extra = (struct tftprecv_extra *)recv_extra;
+	extra = recv_extra;
 	h = extra->tftp_handle;
 
 	len = readudp(d, &ptr, (void **)&t, tleft);
@@ -205,28 +204,36 @@ recvtftp(struct iodesc *d, void **pkt, void **payload,
 	case DATA: {
 		int got;
 
-		if (htons(t->th_block) != (u_short) d->xid) {
+		if (htons(t->th_block) < (u_short)d->xid) {
 			/*
-			 * Expected block?
+			 * Apparently our ACK was missed, re-send.
 			 */
+			tftp_sendack(h, htons(t->th_block));
 			free(ptr);
 			return (-1);
 		}
+		if (htons(t->th_block) != (u_short)d->xid) {
+			/*
+			 * Packet from the future, drop this.
+			 */
+			free(ptr);
+			return (-1);
+		}
 		if (d->xid == 1) {
 			/*
 			 * First data packet from new port.
 			 */
 			struct udphdr *uh;
-			uh = (struct udphdr *) t - 1;
+			uh = (struct udphdr *)t - 1;
 			d->destport = uh->uh_sport;
-		} /* else check uh_sport has not changed??? */
+		}
 		got = len - (t->th_data - (char *)t);
 		*pkt = ptr;
 		*payload = t;
 		return (got);
 	}
 	case ERROR:
-		if ((unsigned) ntohs(t->th_code) > TFTP_MAX_ERRCODE) {
+		if ((unsigned)ntohs(t->th_code) > TFTP_MAX_ERRCODE) {
 			printf("illegal tftp error %d\n", ntohs(t->th_code));
 			errno = EIO;
 		} else {
@@ -241,8 +248,8 @@ recvtftp(struct iodesc *d, void **pkt, void **payload,
 		struct udphdr *uh;
 		int tftp_oack_len;
 
-		/* 
-		 * Unexpected OACK. TFTP transfer already in progress. 
+		/*
+		 * Unexpected OACK. TFTP transfer already in progress.
 		 * Drop the pkt.
 		 */
 		if (d->xid != 1) {
@@ -254,9 +261,9 @@ recvtftp(struct iodesc *d, void **pkt, void **payload,
 		 * Remember which port this OACK came from, because we need
 		 * to send the ACK or errors back to it.
 		 */
-		uh = (struct udphdr *) t - 1;
+		uh = (struct udphdr *)t - 1;
 		d->destport = uh->uh_sport;
-		
+
 		/* Parse options ACK-ed by the server. */
 		tftp_oack_len = len - sizeof(t->th_opcode);
 		if (tftp_parse_oack(h, t->th_u.tu_stuff, tftp_oack_len) != 0) {
@@ -288,9 +295,9 @@ tftp_makereq(struct tftp_handle *h)
 		u_char space[FNAME_SIZE + 6];
 	} __packed __aligned(4) wbuf;
 	struct tftprecv_extra recv_extra;
-	char           *wtail;
-	int             l;
-	ssize_t         res;
+	char *wtail;
+	int l;
+	ssize_t res;
 	void *pkt;
 	struct tftphdr *t;
 	char *tftp_blksize = NULL;
@@ -304,7 +311,7 @@ tftp_makereq(struct tftp_handle *h)
 		tftp_set_blksize(h, tftp_blksize);
 	}
 
-	wbuf.t.th_opcode = htons((u_short) RRQ);
+	wbuf.t.th_opcode = htons((u_short)RRQ);
 	wtail = wbuf.t.th_stuff;
 	l = strlen(h->path);
 #ifdef TFTP_PREPEND_PATH
@@ -329,7 +336,6 @@ tftp_makereq(struct tftp_handle *h)
 	bcopy("0", wtail, 2);
 	wtail += 2;
 
-	/* h->iodesc->myport = htons(--tftpport); */
 	h->iodesc->myport = htons(tftpport + (getsecs() & 0x3ff));
 	h->iodesc->destport = htons(IPPORT_TFTP);
 	h->iodesc->xid = 1;	/* expected block */
@@ -340,8 +346,8 @@ tftp_makereq(struct tftp_handle *h)
 
 	pkt = NULL;
 	recv_extra.tftp_handle = h;
-	res = sendrecv(h->iodesc, &sendudp, &wbuf.t, wtail - (char *) &wbuf.t,
-		       (void *)&recvtftp, &pkt, (void **)&t, &recv_extra);
+	res = sendrecv(h->iodesc, &sendudp, &wbuf.t, wtail - (char *)&wbuf.t,
+	    &recvtftp, &pkt, (void **)&t, &recv_extra);
 	if (res == -1) {
 		free(pkt);
 		return (errno);
@@ -364,7 +370,7 @@ tftp_makereq(struct tftp_handle *h)
 			h->islastblock = 0;
 			if (res < h->tftp_blksize) {
 				h->islastblock = 1;	/* very short file */
-				tftp_sendack(h);
+				tftp_sendack(h, h->currblock);
 			}
 			return (0);
 		}
@@ -376,7 +382,7 @@ tftp_makereq(struct tftp_handle *h)
 }
 
 /* ack block, expect next */
-static int 
+static int
 tftp_getnextblock(struct tftp_handle *h)
 {
 	struct {
@@ -384,21 +390,22 @@ tftp_getnextblock(struct tftp_handle *h)
 		struct tftphdr t;
 	} __packed __aligned(4) wbuf;
 	struct tftprecv_extra recv_extra;
-	char           *wtail;
-	int             res;
+	char *wtail;
+	int res;
 	void *pkt;
 	struct tftphdr *t;
-	wbuf.t.th_opcode = htons((u_short) ACK);
-	wtail = (char *) &wbuf.t.th_block;
-	wbuf.t.th_block = htons((u_short) h->currblock);
+
+	wbuf.t.th_opcode = htons((u_short)ACK);
+	wtail = (char *)&wbuf.t.th_block;
+	wbuf.t.th_block = htons((u_short)h->currblock);
 	wtail += 2;
 
 	h->iodesc->xid = h->currblock + 1;	/* expected block */
 
 	pkt = NULL;
 	recv_extra.tftp_handle = h;
-	res = sendrecv(h->iodesc, &sendudp, &wbuf.t, wtail - (char *) &wbuf.t,
-		       (void *)&recvtftp, &pkt, (void **)&t, &recv_extra);
+	res = sendrecv(h->iodesc, &sendudp, &wbuf.t, wtail - (char *)&wbuf.t,
+	    &recvtftp, &pkt, (void **)&t, &recv_extra);
 
 	if (res == -1) {		/* 0 is OK! */
 		free(pkt);
@@ -414,8 +421,8 @@ tftp_getnextblock(struct tftp_handle *h)
 		h->islastblock = 1;	/* EOF */
 
 	if (h->islastblock == 1) {
-		/* Send an ACK for the last block */ 
-		wbuf.t.th_block = htons((u_short) h->currblock);
+		/* Send an ACK for the last block */
+		wbuf.t.th_block = htons((u_short)h->currblock);
 		sendudp(h->iodesc, &wbuf.t, wtail - (char *)&wbuf.t);
 	}
 
@@ -426,10 +433,10 @@ static int
 tftp_open(const char *path, struct open_file *f)
 {
 	struct tftp_handle *tftpfile;
-	struct iodesc  *io;
-	int             res;
-	size_t          pathsize;
-	const char     *extraslash;
+	struct iodesc	*io;
+	int		res;
+	size_t		pathsize;
+	const char	*extraslash;
 
 	if (netproto != NET_TFTP)
 		return (EINVAL);
@@ -440,13 +447,12 @@ tftp_open(const char *path, struct open_file *f)
 	if (is_open)
 		return (EBUSY);
 
-	tftpfile = (struct tftp_handle *) malloc(sizeof(*tftpfile));
+	tftpfile = calloc(1, sizeof(*tftpfile));
 	if (!tftpfile)
 		return (ENOMEM);
 
-	memset(tftpfile, 0, sizeof(*tftpfile));
 	tftpfile->tftp_blksize = TFTP_REQUESTED_BLKSIZE;
-	tftpfile->iodesc = io = socktodesc(*(int *) (f->f_devdata));
+	tftpfile->iodesc = io = socktodesc(*(int *)(f->f_devdata));
 	if (io == NULL) {
 		free(tftpfile);
 		return (EINVAL);
@@ -458,7 +464,7 @@ tftp_open(const char *path, struct open_file *f)
 	tftpfile->path = malloc(pathsize);
 	if (tftpfile->path == NULL) {
 		free(tftpfile);
-		return(ENOMEM);
+		return (ENOMEM);
 	}
 	if (rootpath[strlen(rootpath) - 1] == '/' || path[0] == '/')
 		extraslash = "";
@@ -469,7 +475,7 @@ tftp_open(const char *path, struct open_file *f)
 	if (res < 0 || res > pathsize) {
 		free(tftpfile->path);
 		free(tftpfile);
-		return(ENOMEM);
+		return (ENOMEM);
 	}
 
 	res = tftp_makereq(tftpfile);
@@ -480,7 +486,7 @@ tftp_open(const char *path, struct open_file *f)
 		free(tftpfile);
 		return (res);
 	}
-	f->f_fsdata = (void *) tftpfile;
+	f->f_fsdata = tftpfile;
 	is_open = 1;
 	return (0);
 }
@@ -490,11 +496,19 @@ tftp_read(struct open_file *f, void *addr, size_t size
     size_t *resid /* out */)
 {
 	struct tftp_handle *tftpfile;
+	size_t res;
 	int rc;
 
 	rc = 0;
-	tftpfile = (struct tftp_handle *) f->f_fsdata;
+	res = size;
+	tftpfile = f->f_fsdata;
 
+	/* Make sure we will not read past file end */
+	if (tftpfile->tftp_tsize > 0 &&
+	    tftpfile->off + size > tftpfile->tftp_tsize) {
+		size = tftpfile->tftp_tsize - tftpfile->off;
+	}
+
 	while (size > 0) {
 		int needblock, count;
 
@@ -542,6 +556,7 @@ tftp_read(struct open_file *f, void *addr, size_t size
 			addr = (char *)addr + count;
 			tftpfile->off += count;
 			size -= count;
+			res -= count;
 
 			if ((tftpfile->islastblock) && (count == inbuffer))
 				break;	/* EOF */
@@ -554,16 +569,16 @@ tftp_read(struct open_file *f, void *addr, size_t size
 
 	}
 
-	if (resid)
-		*resid = size;
+	if (resid != NULL)
+		*resid = res;
 	return (rc);
 }
 
-static int 
+static int
 tftp_close(struct open_file *f)
 {
 	struct tftp_handle *tftpfile;
-	tftpfile = (struct tftp_handle *) f->f_fsdata;
+	tftpfile = f->f_fsdata;
 
 	/* let it time out ... */
 
@@ -576,17 +591,17 @@ tftp_close(struct open_file *f)
 	return (0);
 }
 
-static int 
+static int
 tftp_stat(struct open_file *f, struct stat *sb)
 {
 	struct tftp_handle *tftpfile;
-	tftpfile = (struct tftp_handle *) f->f_fsdata;
+	tftpfile = f->f_fsdata;
 
 	sb->st_mode = 0444 | S_IFREG;
 	sb->st_nlink = 1;
 	sb->st_uid = 0;
 	sb->st_gid = 0;
-	sb->st_size = (off_t) tftpfile->tftp_tsize;
+	sb->st_size = tftpfile->tftp_tsize;
 	return (0);
 }
 
@@ -594,7 +609,7 @@ static off_t
 tftp_seek(struct open_file *f, off_t offset, int where)
 {
 	struct tftp_handle *tftpfile;
-	tftpfile = (struct tftp_handle *) f->f_fsdata;
+	tftpfile = f->f_fsdata;
 
 	switch (where) {
 	case SEEK_SET:
@@ -613,7 +628,7 @@ tftp_seek(struct open_file *f, off_t offset, int where
 static int
 tftp_set_blksize(struct tftp_handle *h, const char *str)
 {
-        char *endptr;
+	char *endptr;
 	int new_blksize;
 	int ret = 0;
 
@@ -628,8 +643,8 @@ tftp_set_blksize(struct tftp_handle *h, const char *st
 	 * RFC2348 specifies that acceptable values are 8-65464.
 	 * Let's choose a limit less than MAXRSPACE.
 	 */
-	if (*endptr == '\0' && new_blksize >= 8
-	    && new_blksize <= TFTP_MAX_BLKSIZE) {
+	if (*endptr == '\0' && new_blksize >= 8 &&
+	    new_blksize <= TFTP_MAX_BLKSIZE) {
 		h->tftp_blksize = new_blksize;
 		ret = 1;
 	}
@@ -660,10 +675,10 @@ tftp_set_blksize(struct tftp_handle *h, const char *st
  *    optN, valueN
  *       The final option/value acknowledgment pair.
  */
-static int 
+static int
 tftp_parse_oack(struct tftp_handle *h, char *buf, size_t len)
 {
-	/* 
+	/*
 	 *  We parse the OACK strings into an array
 	 *  of name-value pairs.
 	 */
@@ -673,7 +688,7 @@ tftp_parse_oack(struct tftp_handle *h, char *buf, size
 	int option_idx = 0;
 	int blksize_is_set = 0;
 	int tsize = 0;
-	
+
 	unsigned int orig_blksize;
 
 	while (option_idx < 128 && i < len) {
@@ -690,26 +705,30 @@ tftp_parse_oack(struct tftp_handle *h, char *buf, size
 	/* Save the block size we requested for sanity check later. */
 	orig_blksize = h->tftp_blksize;
 
-	/* 
+	/*
 	 * Parse individual TFTP options.
 	 *    * "blksize" is specified in RFC2348.
 	 *    * "tsize" is specified in RFC2349.
-	 */ 
+	 */
 	for (i = 0; i < option_idx; i += 2) {
-	    if (strcasecmp(tftp_options[i], "blksize") == 0) {
-		if (i + 1 < option_idx)
-			blksize_is_set =
-			    tftp_set_blksize(h, tftp_options[i + 1]);
-	    } else if (strcasecmp(tftp_options[i], "tsize") == 0) {
-		if (i + 1 < option_idx)
-			tsize = strtol(tftp_options[i + 1], (char **)NULL, 10);
-		if (tsize != 0)
-			h->tftp_tsize = tsize;
-	    } else {
-		/* Do not allow any options we did not expect to be ACKed. */
-		printf("unexpected tftp option '%s'\n", tftp_options[i]);
-		return (-1);
-	    }
+		if (strcasecmp(tftp_options[i], "blksize") == 0) {
+			if (i + 1 < option_idx)
+				blksize_is_set =
+				    tftp_set_blksize(h, tftp_options[i + 1]);
+		} else if (strcasecmp(tftp_options[i], "tsize") == 0) {
+			if (i + 1 < option_idx)
+				tsize = strtol(tftp_options[i + 1], NULL, 10);
+			if (tsize != 0)
+				h->tftp_tsize = tsize;
+		} else {
+			/*
+			 * Do not allow any options we did not expect to be
+			 * ACKed.
+			 */
+			printf("unexpected tftp option '%s'\n",
+			    tftp_options[i]);
+			return (-1);
+		}
 	}
 
 	if (!blksize_is_set) {
@@ -731,5 +750,5 @@ tftp_parse_oack(struct tftp_handle *h, char *buf, size
 	printf("tftp_blksize: %u\n", h->tftp_blksize);
 	printf("tftp_tsize: %lu\n", h->tftp_tsize);
 #endif
-	return 0;
+	return (0);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811291212.wATCCgxc095327>