git: 986cceda16d8 - stable/14 - tftpd: Use `size_t` where appropriate.

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Thu, 02 May 2024 09:06:25 UTC
The branch stable/14 has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=986cceda16d8de091524314b50325d52fe403d75

commit 986cceda16d8de091524314b50325d52fe403d75
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2024-04-25 18:35:15 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2024-05-02 09:02:21 +0000

    tftpd: Use `size_t` where appropriate.
    
    * Limit the use of `ssize_t` to only where it's needed.
    * Correct one case of `int` being used for a length.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D44954
    
    (cherry picked from commit 1ed44fcc44b2c04db330663589541608135402f4)
    
    tftpd: Clean up the tests.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D44955
    
    (cherry picked from commit 7ab7ecfcfe777f7816e3e01df5f277060b2b609a)
    
    tftpd: Check the server status after each test.
    
    * In the setup phase, wait for the server to start (or fail to start)
      before proceeding with the test.  This makes it possible to write test
      cases that don't expect a response from the server without ending up
      in a race over the server PID file.
    * After running each test, wait up to 30 seconds for the server to exit
      and check that the exit status matches what the test case says to
      expect (usually 0).
    * We still kill and collect the server in the cleanup phase, in case the
      test ended early.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D44956
    
    (cherry picked from commit 83a6e984ac01657819418746f722163367ec30db)
    
    tftpd: Immediately reject any request shorter than 4 bytes.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D44957
    
    (cherry picked from commit 9f231af307b80eb222d9761bbd81fa4e130bb3d7)
    
    tftpd: Untangle a conditional.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    markj
    Differential Revision:  https://reviews.freebsd.org/D45026
    
    (cherry picked from commit 21b5829d28b1e02e9f30bfa439238c382dd19114)
---
 libexec/tftpd/tests/functional.c | 352 +++++++++++++++++++++++----------------
 libexec/tftpd/tftp-utils.c       |   4 +-
 libexec/tftpd/tftp-utils.h       |   2 +-
 libexec/tftpd/tftpd.c            |  30 ++--
 4 files changed, 233 insertions(+), 155 deletions(-)

diff --git a/libexec/tftpd/tests/functional.c b/libexec/tftpd/tests/functional.c
index 1d5c1e17c420..f31d2a893da1 100644
--- a/libexec/tftpd/tests/functional.c
+++ b/libexec/tftpd/tests/functional.c
@@ -52,8 +52,8 @@ static bool s_flag = false;	/* Pass -s to tftpd */
 static bool w_flag = false;	/* Pass -w to tftpd */
 
 /* Helper functions*/
-static void require_bufeq(const char *expected, ssize_t expected_len,
-    const char *actual, ssize_t len);
+static void require_bufeq(const char *expected, size_t expected_len,
+    const char *actual, size_t len);
 
 /*
  * Receive a response from tftpd
@@ -61,24 +61,24 @@ static void require_bufeq(const char *expected, ssize_t expected_len,
  * @param	contents	The reply's expected contents, as a char array
  * @param	contents_len	Length of contents
  */
-#define RECV(hdr, contents, contents_len) do { \
-	char buffer[1024]; \
-	struct sockaddr_storage from; \
-	socklen_t fromlen = sizeof(from); \
-	ssize_t r = recvfrom(s, buffer, sizeof(buffer), 0, \
-	    (struct sockaddr*)&from, &fromlen); \
-	ATF_REQUIRE(r > 0); \
-	require_bufeq((hdr), sizeof(hdr), buffer, \
-	    MIN(r, (ssize_t)sizeof(hdr))); \
-	require_bufeq((const char*) (contents), (contents_len), \
-	    &buffer[sizeof(hdr)], r - sizeof(hdr)); \
-	if (protocol == PF_INET) { \
-		((struct sockaddr_in*)&addr)->sin_port = \
-		    ((struct sockaddr_in*)&from)->sin_port; \
-	} else { \
-		((struct sockaddr_in6*)&addr)->sin6_port = \
-		    ((struct sockaddr_in6*)&from)->sin6_port; \
-	} \
+#define RECV(hdr, contents, contents_len) do {				\
+	char buffer[1024];						\
+	struct sockaddr_storage from;					\
+	socklen_t fromlen = sizeof(from);				\
+	ssize_t r = recvfrom(s, buffer, sizeof(buffer), 0,		\
+	    (struct sockaddr *)&from, &fromlen);			\
+	ATF_REQUIRE(r > 0);						\
+	require_bufeq((hdr), sizeof(hdr), buffer,			\
+	    MIN((size_t)r, sizeof(hdr)));				\
+	require_bufeq((const char *) (contents), (contents_len),	\
+	    &buffer[sizeof(hdr)], r - sizeof(hdr));			\
+	if (protocol == PF_INET) {					\
+		((struct sockaddr_in *)&addr)->sin_port =		\
+		    ((struct sockaddr_in *)&from)->sin_port;		\
+	} else {							\
+		((struct sockaddr_in6 *)&addr)->sin6_port =		\
+		    ((struct sockaddr_in6 *)&from)->sin6_port;		\
+	}								\
 } while(0)
 
 static void
@@ -102,15 +102,15 @@ recv_oack(const char *options, size_t options_len)
  * @param	contents_len	Length of contents expected to receive
  */
 static void
-recv_data(uint16_t blocknum, const char* contents, size_t contents_len)
+recv_data(uint16_t blocknum, const char *contents, size_t contents_len)
 {
 	char hdr[] = {0, 3, blocknum >> 8, blocknum & 0xFF};
 	RECV(hdr, contents, contents_len);
 }
 
-#define RECV_ERROR(code, msg) do { \
-	char hdr[] = {0, 5, code >> 8, code & 0xFF}; \
-	RECV(hdr, msg, sizeof(msg)); \
+#define RECV_ERROR(code, msg) do {					\
+	char hdr[] = {0, 5, code >> 8, code & 0xFF};			\
+	RECV(hdr, msg, sizeof(msg));					\
 } while (0)
 
 /* 
@@ -118,16 +118,17 @@ recv_data(uint16_t blocknum, const char* contents, size_t contents_len)
  * @param	cmd		Command to send, as a char array
  */
 static void
-send_bytes(const void* cmd, ssize_t len)
+send_bytes(const void *cmd, size_t len)
 {
 	ssize_t r;
 
-	r = sendto(s, cmd, len, 0, (struct sockaddr*)(&addr), addr.ss_len);
-	ATF_REQUIRE_EQ(r, len);
+	r = sendto(s, cmd, len, 0, (struct sockaddr *)(&addr), addr.ss_len);
+	ATF_REQUIRE(r >= 0);
+	ATF_REQUIRE_EQ(len, (size_t)r);
 }
 
 static void
-send_data(uint16_t blocknum, const char* contents, size_t contents_len)
+send_data(uint16_t blocknum, const char *contents, size_t contents_len)
 {
 	char buffer[1024];
 
@@ -144,10 +145,10 @@ send_data(uint16_t blocknum, const char* contents, size_t contents_len)
  * @param	cmd		Command to send, as a const string
  *				(terminating NUL will be ignored)
  */
-#define SEND_STR(cmd) ATF_REQUIRE_EQ( \
-	sendto(s, (cmd), sizeof(cmd) - 1, 0, (struct sockaddr*)(&addr), \
-	    addr.ss_len), \
-	sizeof(cmd) - 1)
+#define SEND_STR(cmd)							\
+	ATF_REQUIRE_EQ(sizeof(cmd) - 1,					\
+	    sendto(s, (cmd), sizeof(cmd) - 1, 0,			\
+	    (struct sockaddr *)(&addr), addr.ss_len))
 
 /*
  * Acknowledge block blocknum
@@ -156,13 +157,12 @@ static void
 send_ack(uint16_t blocknum)
 {
 	char packet[] = {
-	    0, 4,	/* ACK opcode in BE */
-	    blocknum >> 8,
-	    blocknum & 0xFF
+		0, 4,		/* ACK opcode in BE */
+		blocknum >> 8,
+		blocknum & 0xFF
 	};
 
 	send_bytes(packet, sizeof(packet));
-
 }
 
 /*
@@ -175,73 +175,109 @@ send_ack(uint16_t blocknum)
  * @param	filename	filename as a string, absolute or relative
  * @param	mode		either "octet" or "netascii"
  */
-#define SEND_RRQ(filename, mode) SEND_STR("\0\001" filename "\0" mode "\0")
+#define SEND_RRQ(filename, mode)					\
+	SEND_STR("\0\001" filename "\0" mode "\0")
 
 /*
  * send a read request with options
  */
-#define SEND_RRQ_OPT(filename, mode, options) SEND_STR("\0\001" filename "\0" mode "\000" options)
+#define SEND_RRQ_OPT(filename, mode, options)				\
+	SEND_STR("\0\001" filename "\0" mode "\000" options)
 
 /* 
  * send a write request to tftpd.
  * @param	filename	filename as a string, absolute or relative
  * @param	mode		either "octet" or "netascii"
  */
-#define SEND_WRQ(filename, mode) SEND_STR("\0\002" filename "\0" mode "\0")
+#define SEND_WRQ(filename, mode)					\
+	SEND_STR("\0\002" filename "\0" mode "\0")
 
 /*
  * send a write request with options
  */
-#define SEND_WRQ_OPT(filename, mode, options) SEND_STR("\0\002" filename "\0" mode "\000" options)
+#define SEND_WRQ_OPT(filename, mode, options)				\
+	SEND_STR("\0\002" filename "\0" mode "\000" options)
 
 /* Define a test case, for both IPv4 and IPv6 */
-#define TFTPD_TC_DEFINE(name, head, ...) \
-static void \
-name ## _body(void); \
-ATF_TC_WITH_CLEANUP(name ## _v4); \
-ATF_TC_HEAD(name ## _v4, tc) \
-{ \
-	head \
-} \
-ATF_TC_BODY(name ## _v4, tc) \
-{ \
-	__VA_ARGS__; \
-	protocol = AF_INET; \
-	s = setup(&addr, __COUNTER__); \
-	name ## _body(); \
-	close(s); \
-} \
-ATF_TC_CLEANUP(name ## _v4, tc) \
-{ \
-	cleanup(); \
-} \
-ATF_TC_WITH_CLEANUP(name ## _v6); \
-ATF_TC_HEAD(name ## _v6, tc) \
-{ \
-	head \
-} \
-ATF_TC_BODY(name ## _v6, tc) \
-{ \
-	__VA_ARGS__; \
-	protocol = AF_INET6; \
-	s = setup(&addr, __COUNTER__); \
-	name ## _body(); \
-	close(s); \
-} \
-ATF_TC_CLEANUP(name ## _v6, tc) \
-{ \
-	cleanup(); \
-} \
-static void \
+#define TFTPD_TC_DEFINE(name, head, ...)				\
+static void								\
+name ## _body(void);							\
+ATF_TC_WITH_CLEANUP(name ## _v4);					\
+ATF_TC_HEAD(name ## _v4, tc)						\
+{									\
+	head								\
+}									\
+ATF_TC_BODY(name ## _v4, tc)						\
+{									\
+	int exitcode = 0;						\
+	__VA_ARGS__;							\
+	protocol = AF_INET;						\
+	s = setup(&addr, __COUNTER__);					\
+	name ## _body();						\
+	close(s);							\
+	if (exitcode >= 0)						\
+		check_server(exitcode);					\
+}									\
+ATF_TC_CLEANUP(name ## _v4, tc)						\
+{									\
+	cleanup();							\
+}									\
+ATF_TC_WITH_CLEANUP(name ## _v6);					\
+ATF_TC_HEAD(name ## _v6, tc)						\
+{									\
+	head								\
+}									\
+ATF_TC_BODY(name ## _v6, tc)						\
+{									\
+	int exitcode = 0;						\
+	__VA_ARGS__;							\
+	protocol = AF_INET6;						\
+	s = setup(&addr, __COUNTER__);					\
+	name ## _body();						\
+	close(s);							\
+	if (exitcode >= 0)						\
+		check_server(exitcode);					\
+}									\
+ATF_TC_CLEANUP(name ## _v6, tc)						\
+{									\
+	cleanup();							\
+}									\
+static void								\
 name ## _body(void)
 
 /* Add the IPv4 and IPv6 versions of a test case */
-#define TFTPD_TC_ADD(tp, name ) \
-do { \
-	ATF_TP_ADD_TC(tp, name ## _v4); \
-	ATF_TP_ADD_TC(tp, name ## _v6); \
+#define TFTPD_TC_ADD(tp, name) do {					\
+	ATF_TP_ADD_TC(tp, name ## _v4);					\
+	ATF_TP_ADD_TC(tp, name ## _v6);					\
 } while (0)
 
+static void
+sigalrm(int signo __unused)
+{
+}
+
+/* Check that server exits with specific exit code */
+static void
+check_server(int exitcode)
+{
+	struct sigaction sa = { .sa_handler = sigalrm };
+	struct itimerval it = { .it_value = { .tv_sec = 30 } };
+	FILE *f;
+	pid_t pid;
+	int wstatus;
+
+	f = fopen(pidfile, "r");
+	ATF_REQUIRE(f != NULL);
+	ATF_REQUIRE_INTEQ(1, fscanf(f, "%d", &pid));
+	ATF_CHECK_INTEQ(0, fclose(f));
+	ATF_REQUIRE_INTEQ(0, sigaction(SIGALRM, &sa, NULL));
+	ATF_REQUIRE_EQ(0, setitimer(ITIMER_REAL, &it, NULL));
+	ATF_REQUIRE_EQ(pid, waitpid(pid, &wstatus, 0));
+	ATF_CHECK(WIFEXITED(wstatus));
+	ATF_CHECK_INTEQ(exitcode, WEXITSTATUS(wstatus));
+	unlink(pidfile);
+}
+
 /* Standard cleanup used by all testcases */
 static void
 cleanup(void)
@@ -252,26 +288,26 @@ cleanup(void)
 	f = fopen(pidfile, "r");
 	if (f == NULL)
 		return;
+	unlink(pidfile);
 	if (fscanf(f, "%d", &pid) == 1) {
 		kill(pid, SIGTERM);
 		waitpid(pid, NULL, 0);
 	}
 	fclose(f);
-	unlink(pidfile);
 }
 
 /* Assert that two binary buffers are identical */
 static void
-require_bufeq(const char *expected, ssize_t expected_len, const char *actual,
-    ssize_t len)
+require_bufeq(const char *expected, size_t expected_len,
+    const char *actual, size_t len)
 {
-	ssize_t i;
+	size_t i;
 
 	ATF_REQUIRE_EQ_MSG(expected_len, len,
-	    "Expected %zd bytes but got %zd", expected_len, len);
+	    "Expected %zu bytes but got %zu", expected_len, len);
 	for (i = 0; i < len; i++) {
-		ATF_REQUIRE_EQ_MSG(actual[i], expected[i],
-		    "Expected %#hhx at position %zd; got %hhx instead",
+		ATF_REQUIRE_EQ_MSG(expected[i], actual[i],
+		    "Expected %#hhx at position %zu; got %hhx instead",
 		    expected[i], i, actual[i]);
 	}
 }
@@ -297,6 +333,9 @@ setup(struct sockaddr_storage *to, uint16_t idx)
 	struct pidfh *pfh;
 	uint16_t port = BASEPORT + idx;
 	socklen_t len;
+	int pd[2];
+
+	ATF_REQUIRE_EQ(0, pipe2(pd, O_CLOEXEC));
 
 	if (protocol == PF_INET) {
 		len = sizeof(addr4);
@@ -304,17 +343,17 @@ setup(struct sockaddr_storage *to, uint16_t idx)
 		addr4.sin_len = len;
 		addr4.sin_family = PF_INET;
 		addr4.sin_port = htons(port);
-		server_addr = (struct sockaddr*)&addr4;
+		server_addr = (struct sockaddr *)&addr4;
 	} else {
 		len = sizeof(addr6);
 		bzero(&addr6, len);
 		addr6.sin6_len = len;
 		addr6.sin6_family = PF_INET6;
 		addr6.sin6_port = htons(port);
-		server_addr = (struct sockaddr*)&addr6;
+		server_addr = (struct sockaddr *)&addr6;
 	}
 
-	ATF_REQUIRE_EQ(getcwd(pwd, sizeof(pwd)), pwd);
+	ATF_REQUIRE_EQ(pwd, getcwd(pwd, sizeof(pwd)));
 	
 	/* Must bind(2) pre-fork so it happens before the client's send(2) */
 	server_s = socket(protocol, SOCK_DGRAM, 0);
@@ -324,7 +363,7 @@ setup(struct sockaddr_storage *to, uint16_t idx)
 	}
 	ATF_REQUIRE_MSG(server_s >= 0,
 	    "socket failed with error %s", strerror(errno));
-	ATF_REQUIRE_EQ_MSG(bind(server_s, server_addr, len), 0,
+	ATF_REQUIRE_EQ_MSG(0, bind(server_s, server_addr, len),
 	    "bind failed with error %s", strerror(errno));
 
 	pid = fork();
@@ -337,8 +376,8 @@ setup(struct sockaddr_storage *to, uint16_t idx)
 		pfh = pidfile_open(pidfile, 0644, NULL);
 		ATF_REQUIRE_MSG(pfh != NULL,
 		    "pidfile_open: %s", strerror(errno));
-		ATF_REQUIRE_EQ(pidfile_write(pfh), 0);
-		ATF_REQUIRE_EQ(pidfile_close(pfh), 0);
+		ATF_REQUIRE_EQ(0, pidfile_write(pfh));
+		ATF_REQUIRE_EQ(0, pidfile_close(pfh));
 
 		bzero(argv, sizeof(argv));
 		argv[0] = execname;
@@ -348,31 +387,35 @@ setup(struct sockaddr_storage *to, uint16_t idx)
 		if (s_flag)
 			argv[argv_idx++] = s_flag_str;
 		argv[argv_idx++] = pwd;
-		ATF_REQUIRE_EQ(dup2(server_s, STDOUT_FILENO), STDOUT_FILENO);
-		ATF_REQUIRE_EQ(dup2(server_s, STDIN_FILENO), STDIN_FILENO);
-		ATF_REQUIRE_EQ(dup2(server_s, STDERR_FILENO), STDERR_FILENO);
+		ATF_REQUIRE_EQ(STDOUT_FILENO, dup2(server_s, STDOUT_FILENO));
+		ATF_REQUIRE_EQ(STDIN_FILENO, dup2(server_s, STDIN_FILENO));
+		ATF_REQUIRE_EQ(STDERR_FILENO, dup2(server_s, STDERR_FILENO));
 		execv(execname, argv);
 		atf_tc_fail("exec failed");
 		break;
 	default:
 		/* In parent */
+		ATF_REQUIRE_INTEQ(0, close(pd[1]));
+		/* block until other end is closed on exec() or exit() */
+		ATF_REQUIRE_INTEQ(0, read(pd[0], &pd[1], sizeof(pd[1])));
+		ATF_REQUIRE_INTEQ(0, close(pd[0]));
 		bzero(to, sizeof(*to));
 		if (protocol == PF_INET) {
-			struct sockaddr_in *to4 = (struct sockaddr_in*)to;
+			struct sockaddr_in *to4 = (struct sockaddr_in *)to;
 			to4->sin_len = sizeof(*to4);
 			to4->sin_family = PF_INET;
 			to4->sin_port = htons(port);
 			to4->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
 		} else {
 			struct in6_addr loopback = IN6ADDR_LOOPBACK_INIT;
-			struct sockaddr_in6 *to6 = (struct sockaddr_in6*)to;
+			struct sockaddr_in6 *to6 = (struct sockaddr_in6 *)to;
 			to6->sin6_len = sizeof(*to6);
 			to6->sin6_family = PF_INET6;
 			to6->sin6_port = htons(port);
 			to6->sin6_addr = loopback;
 		}
 
-		close(server_s);
+		ATF_REQUIRE_INTEQ(0, close(server_s));
 		ATF_REQUIRE((client_s = socket(protocol, SOCK_DGRAM, 0)) > 0);
 		break;
 	}
@@ -392,8 +435,8 @@ write_all(int fd, const void *buf, size_t nbytes)
 	while (nbytes > 0) {
 		r = write(fd, buf, nbytes);
 		ATF_REQUIRE(r > 0);
-		nbytes -= r;
-		buf = (const char*)buf + r;
+		nbytes -= (size_t)r;
+		buf = (const char *)buf + (size_t)r;
 	}
 }
 
@@ -433,13 +476,13 @@ TFTPD_TC_DEFINE(abspath,)
  */
 TFTPD_TC_DEFINE(dotdot,)
 {
-	ATF_REQUIRE_EQ(mkdir("subdir", 0777), 0);
+	ATF_REQUIRE_EQ(0, mkdir("subdir", 0777));
 	SEND_RRQ("../disallowed.txt", "octet");
 	RECV_ERROR(2, "Access violation");
-	s = setup(&addr, __COUNTER__); \
+	s = setup(&addr, __COUNTER__);
 	SEND_RRQ("subdir/../../disallowed.txt", "octet");
 	RECV_ERROR(2, "Access violation");
-	s = setup(&addr, __COUNTER__); \
+	s = setup(&addr, __COUNTER__);
 	SEND_RRQ("/etc/passwd", "octet");
 	RECV_ERROR(2, "Access violation");
 }
@@ -447,8 +490,9 @@ TFTPD_TC_DEFINE(dotdot,)
 /*
  * With "-s", tftpd should chroot to the specified directory
  */
-TFTPD_TC_DEFINE(s_flag, atf_tc_set_md_var(tc, "require.user", "root");,
-		s_flag = true)
+TFTPD_TC_DEFINE(s_flag,
+    atf_tc_set_md_var(tc, "require.user", "root");,
+    s_flag = true)
 {
 	int fd;
 	char contents[] = "small";
@@ -505,7 +549,7 @@ TFTPD_TC_DEFINE(rrq_dropped_data,)
 	close(fd);
 
 	SEND_RRQ("medium.txt", "octet");
-	recv_data(1, (const char*)&contents[0], 512);
+	recv_data(1, (const char *)&contents[0], 512);
 	send_ack(1);
 	(void) recvfrom(s, buffer, sizeof(buffer), 0, NULL, NULL);
 	/*
@@ -513,7 +557,7 @@ TFTPD_TC_DEFINE(rrq_dropped_data,)
 	 * Eventually, client should resend the last ACK
 	 */
 	send_ack(1);
-	recv_data(2, (const char*)&contents[128], 256);
+	recv_data(2, (const char *)&contents[128], 256);
 	send_ack(2);
 }
 
@@ -535,11 +579,11 @@ TFTPD_TC_DEFINE(rrq_duped_ack,)
 	close(fd);
 
 	SEND_RRQ("medium.txt", "octet");
-	recv_data(1, (const char*)&contents[0], 512);
+	recv_data(1, (const char *)&contents[0], 512);
 	send_ack(1);
 	send_ack(1);	/* Dupe an ACK packet */
-	recv_data(2, (const char*)&contents[128], 256);
-	recv_data(2, (const char*)&contents[128], 256);
+	recv_data(2, (const char *)&contents[128], 256);
+	recv_data(2, (const char *)&contents[128], 256);
 	send_ack(2);
 }
 
@@ -593,9 +637,9 @@ TFTPD_TC_DEFINE(rrq_medium,)
 	close(fd);
 
 	SEND_RRQ("medium.txt", "octet");
-	recv_data(1, (const char*)&contents[0], 512);
+	recv_data(1, (const char *)&contents[0], 512);
 	send_ack(1);
-	recv_data(2, (const char*)&contents[128], 256);
+	recv_data(2, (const char *)&contents[128], 256);
 	send_ack(2);
 }
 
@@ -620,8 +664,8 @@ TFTPD_TC_DEFINE(rrq_medium_window,)
 	SEND_RRQ_OPT("medium.txt", "octet", OPTION_STR("windowsize", "2"));
 	recv_oack(options, sizeof(options) - 1);
 	send_ack(0);
-	recv_data(1, (const char*)&contents[0], 512);
-	recv_data(2, (const char*)&contents[128], 256);
+	recv_data(1, (const char *)&contents[0], 512);
+	recv_data(2, (const char *)&contents[128], 256);
 	send_ack(2);
 }
 
@@ -764,13 +808,13 @@ TFTPD_TC_DEFINE(unknown_modes,)
 {
 	SEND_RRQ("foo.txt", "ascii");	/* Misspelling of "ascii" */
 	RECV_ERROR(4, "Illegal TFTP operation");
-	s = setup(&addr, __COUNTER__); \
+	s = setup(&addr, __COUNTER__);
 	SEND_RRQ("foo.txt", "binary");	/* Obsolete.  Use "octet" instead */
 	RECV_ERROR(4, "Illegal TFTP operation");
-	s = setup(&addr, __COUNTER__); \
+	s = setup(&addr, __COUNTER__);
 	SEND_RRQ("foo.txt", "en_US.UTF-8");
 	RECV_ERROR(4, "Illegal TFTP operation");
-	s = setup(&addr, __COUNTER__); \
+	s = setup(&addr, __COUNTER__);
 	SEND_RRQ("foo.txt", "mail");	/* Obsolete in RFC-1350 */
 	RECV_ERROR(4, "Illegal TFTP operation");
 }
@@ -805,8 +849,9 @@ TFTPD_TC_DEFINE(w_flag,, w_flag = 1;)
 	fd = open("small.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq(contents, contents_len, buffer, r);
+	require_bufeq(contents, contents_len, buffer, (size_t)r);
 }
 
 /*
@@ -829,21 +874,22 @@ TFTPD_TC_DEFINE(wrq_dropped_ack,)
 
 	SEND_WRQ("medium.txt", "octet");
 	recv_ack(0);
-	send_data(1, (const char*)&contents[0], 512);
+	send_data(1, (const char *)&contents[0], 512);
 	/* 
 	 * Servers "sends" an ACK packet, but network drops it.
 	 * Eventually, server should resend the last ACK
 	 */
 	(void) recvfrom(s, buffer, sizeof(buffer), 0, NULL, NULL);
 	recv_ack(1);
-	send_data(2, (const char*)&contents[128], 256);
+	send_data(2, (const char *)&contents[128], 256);
 	recv_ack(2);
 
 	fd = open("medium.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq((const char*)contents, 768, buffer, r);
+	require_bufeq((const char *)contents, 768, buffer, (size_t)r);
 }
 
 /*
@@ -875,8 +921,9 @@ TFTPD_TC_DEFINE(wrq_dropped_data,)
 	fd = open("small.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq(contents, contents_len, buffer, r);
+	require_bufeq(contents, contents_len, buffer, (size_t)r);
 }
 
 /*
@@ -899,18 +946,19 @@ TFTPD_TC_DEFINE(wrq_duped_data,)
 
 	SEND_WRQ("medium.txt", "octet");
 	recv_ack(0);
-	send_data(1, (const char*)&contents[0], 512);
-	send_data(1, (const char*)&contents[0], 512);
+	send_data(1, (const char *)&contents[0], 512);
+	send_data(1, (const char *)&contents[0], 512);
 	recv_ack(1);
 	recv_ack(1);
-	send_data(2, (const char*)&contents[128], 256);
+	send_data(2, (const char *)&contents[128], 256);
 	recv_ack(2);
 
 	fd = open("medium.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq((const char*)contents, 768, buffer, r);
+	require_bufeq((const char *)contents, 768, buffer, (size_t)r);
 }
 
 /*
@@ -965,16 +1013,17 @@ TFTPD_TC_DEFINE(wrq_medium,)
 
 	SEND_WRQ("medium.txt", "octet");
 	recv_ack(0);
-	send_data(1, (const char*)&contents[0], 512);
+	send_data(1, (const char *)&contents[0], 512);
 	recv_ack(1);
-	send_data(2, (const char*)&contents[128], 256);
+	send_data(2, (const char *)&contents[128], 256);
 	recv_ack(2);
 
 	fd = open("medium.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq((const char*)contents, 768, buffer, r);
+	require_bufeq((const char *)contents, 768, buffer, (size_t)r);
 }
 
 /*
@@ -998,15 +1047,16 @@ TFTPD_TC_DEFINE(wrq_medium_window,)
 
 	SEND_WRQ_OPT("medium.txt", "octet", OPTION_STR("windowsize", "2"));
 	recv_oack(options, sizeof(options) - 1);
-	send_data(1, (const char*)&contents[0], 512);
-	send_data(2, (const char*)&contents[128], 256);
+	send_data(1, (const char *)&contents[0], 512);
+	send_data(2, (const char *)&contents[128], 256);
 	recv_ack(2);
 
 	fd = open("medium.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq((const char*)contents, 768, buffer, r);
+	require_bufeq((const char *)contents, 768, buffer, (size_t)r);
 }
 
 /*
@@ -1038,8 +1088,9 @@ TFTPD_TC_DEFINE(wrq_netascii,)
 	fd = open("unix.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq(expected, sizeof(expected), buffer, r);
+	require_bufeq(expected, sizeof(expected), buffer, (size_t)r);
 }
 
 /*
@@ -1076,8 +1127,9 @@ TFTPD_TC_DEFINE(wrq_small,)
 	fd = open("small.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq(contents, contents_len, buffer, r);
+	require_bufeq(contents, contents_len, buffer, (size_t)r);
 }
 
 /*
@@ -1099,8 +1151,8 @@ TFTPD_TC_DEFINE(wrq_truncate,)
 	send_data(1, NULL, 0);
 	recv_ack(1);
 
-	ATF_REQUIRE_EQ(stat("small.txt", &sb), 0);
-	ATF_REQUIRE_EQ(sb.st_size, 0);
+	ATF_REQUIRE_EQ(0, stat("small.txt", &sb));
+	ATF_REQUIRE_EQ(0, sb.st_size);
 }
 
 /*
@@ -1163,8 +1215,25 @@ TFTPD_TC_DEFINE(wrq_window_rfc7440,)
 	fd = open("rfc7440.txt", O_RDONLY);
 	ATF_REQUIRE(fd >= 0);
 	r = read(fd, buffer, sizeof(buffer));
+	ATF_REQUIRE(r > 0);
 	close(fd);
-	require_bufeq(contents, sizeof(contents), buffer, r);
+	require_bufeq(contents, sizeof(contents), buffer, (size_t)r);
+}
+
+/*
+ * Send less than four bytes
+ */
+TFTPD_TC_DEFINE(short_packet1, /* no head */, exitcode = 1)
+{
+	SEND_STR("\1");
+}
+TFTPD_TC_DEFINE(short_packet2, /* no head */, exitcode = 1)
+{
+	SEND_STR("\1\2");
+}
+TFTPD_TC_DEFINE(short_packet3, /* no head */, exitcode = 1)
+{
+	SEND_STR("\1\2\3");
 }
 
 
@@ -1204,6 +1273,9 @@ ATF_TP_ADD_TCS(tp)
 	TFTPD_TC_ADD(tp, wrq_small);
 	TFTPD_TC_ADD(tp, wrq_truncate);
 	TFTPD_TC_ADD(tp, wrq_window_rfc7440);
+	TFTPD_TC_ADD(tp, short_packet1);
+	TFTPD_TC_ADD(tp, short_packet2);
+	TFTPD_TC_ADD(tp, short_packet3);
 
 	return (atf_no_error());
 }
diff --git a/libexec/tftpd/tftp-utils.c b/libexec/tftpd/tftp-utils.c
index 9754c3238d50..b309a94f7653 100644
--- a/libexec/tftpd/tftp-utils.c
+++ b/libexec/tftpd/tftp-utils.c
@@ -104,8 +104,8 @@ unmappedaddr(struct sockaddr_in6 *sin6)
 }
 
 /* Get a field from a \0 separated string */
-ssize_t
-get_field(int peer, char *buffer, ssize_t size)
+size_t
+get_field(int peer, char *buffer, size_t size)
 {
 	char *cp = buffer;
 
diff --git a/libexec/tftpd/tftp-utils.h b/libexec/tftpd/tftp-utils.h
index 3fecf1fc8696..763b3b493c7e 100644
--- a/libexec/tftpd/tftp-utils.h
+++ b/libexec/tftpd/tftp-utils.h
@@ -63,7 +63,7 @@ extern int	acting_as_client;
 /*
  */
 void	unmappedaddr(struct sockaddr_in6 *sin6);
-ssize_t	get_field(int peer, char *buffer, ssize_t size);
+size_t	get_field(int peer, char *buffer, size_t size);
 
 /*
  * Packet types
diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c
index ffe31927780e..78348a8c6aaf 100644
--- a/libexec/tftpd/tftpd.c
+++ b/libexec/tftpd/tftpd.c
@@ -80,8 +80,8 @@ static char sccsid[] = "@(#)tftpd.c	8.1 (Berkeley) 6/4/93";
 #include <tcpd.h>
 #endif
 
-static void	tftp_wrq(int peer, char *, ssize_t);
-static void	tftp_rrq(int peer, char *, ssize_t);
+static void	tftp_wrq(int peer, char *, size_t);
+static void	tftp_rrq(int peer, char *, size_t);
 
 /*
  * Null-terminated directory prefix list for absolute pathname requests and
@@ -93,7 +93,7 @@ static void	tftp_rrq(int peer, char *, ssize_t);
 #define MAXDIRS	20
 static struct dirlist {
 	const char	*name;
-	int	len;
+	size_t		len;
 } dirs[MAXDIRS+1];
 static int	suppress_naks;
 static int	logging;
@@ -240,6 +240,11 @@ main(int argc, char *argv[])
 	}
 	getnameinfo((struct sockaddr *)&peer_sock, peer_sock.ss_len,
 	    peername, sizeof(peername), NULL, 0, NI_NUMERICHOST);
+	if ((size_t)n < 4 /* tftphdr */) {
+		tftp_log(LOG_ERR, "Rejecting %zd-byte request from %s",
+		    n, peername);
+		exit(1);
+	}
 
 	/*
 	 * Now that we have read the message out of the UDP
@@ -404,7 +409,7 @@ main(int argc, char *argv[])
 	tp->th_opcode = ntohs(tp->th_opcode);
 	if (tp->th_opcode == RRQ) {
 		if (allow_ro)
-			tftp_rrq(peer, tp->th_stuff, n - 1);
+			tftp_rrq(peer, tp->th_stuff, (size_t)n - 1);
 		else {
 			tftp_log(LOG_WARNING,
 			    "%s read access denied", peername);
@@ -412,7 +417,7 @@ main(int argc, char *argv[])
 		}
 	} else if (tp->th_opcode == WRQ) {
 		if (allow_wo)
-			tftp_wrq(peer, tp->th_stuff, n - 1);
+			tftp_wrq(peer, tp->th_stuff, (size_t)n - 1);
 		else {
 			tftp_log(LOG_WARNING,
 			    "%s write access denied", peername);
@@ -455,7 +460,7 @@ reduce_path(char *fn)
 }
 
 static char *
-parse_header(int peer, char *recvbuffer, ssize_t size,
+parse_header(int peer, char *recvbuffer, size_t size,
 	char **filename, char **mode)
 {
 	char	*cp;
@@ -501,7 +506,7 @@ parse_header(int peer, char *recvbuffer, ssize_t size,
  * WRQ - receive a file from the client
  */
 void
-tftp_wrq(int peer, char *recvbuffer, ssize_t size)
+tftp_wrq(int peer, char *recvbuffer, size_t size)
 {
 	char *cp;
 	int has_options = 0, ecode;
@@ -546,7 +551,7 @@ tftp_wrq(int peer, char *recvbuffer, ssize_t size)
  * RRQ - send a file to the client
  */
 void
-tftp_rrq(int peer, char *recvbuffer, ssize_t size)
+tftp_rrq(int peer, char *recvbuffer, size_t size)
 {
 	char *cp;
 	int has_options = 0, ecode;
@@ -694,10 +699,11 @@ validate_access(int peer, char **filep, int mode)
 		 * it's a /.
 		 */
 		for (dirp = dirs; dirp->name != NULL; dirp++) {
-			if (dirp->len == 1 ||
-			    (!strncmp(filename, dirp->name, dirp->len) &&
-			     filename[dirp->len] == '/'))
-				    break;
+			if (dirp->len == 1)
+				break;
+			if (strncmp(filename, dirp->name, dirp->len) == 0 &&
+			    filename[dirp->len] == '/')
+				break;
 		}
 		/* If directory list is empty, allow access to any file */
 		if (dirp->name == NULL && dirp != dirs)