socket.c: Add wrapper for sendfile.

sendfile(2) isn't guaranteed to fully write the requested
data, so use our own wrapper that does. This fixes truncation
that would occur when sendfile returned early.

bbs/socket.c

@@ -31,6 +31,7 @@
 #include <netdb.h> /* use getnameinfo */
 #include <ifaddrs.h>
 #include <poll.h>
+#include <sys/sendfile.h>
 
 /* #define DEBUG_TEXT_IO */
 
@@ -2161,6 +2162,29 @@ ssize_t bbs_timed_write(int fd, const char *buf, size_t len, int ms)
 	return res;
 }
 
+ssize_t bbs_sendfile(int out_fd, int in_fd, off_t *offset, size_t count)
+{
+	/* sendfile (like write) can return before writing all the requested bytes.
+	 * This wrapper function will retry as appropriate to fully write the message as best we can. */
+	ssize_t written = 0;
+
+	for (;;) {
+		ssize_t res = sendfile(out_fd, in_fd, offset, count);
+		if (res < 0) {
+			bbs_error("Failed to write %lu bytes, sendfile %d -> %d failed: %s\n", count, in_fd, out_fd, strerror(errno));
+			return res;
+		}
+		written += res;
+		if (res == (ssize_t) count) {
+			break;
+		}
+		bbs_debug(2, "Wanted to write %lu bytes but was only able to write %ld this round\n", count, res); /* Keep trying */
+		count -= (size_t) res;
+	}
+
+	return written;
+}
+
 ssize_t bbs_node_fd_write(struct bbs_node *node, int fd, const char *buf, size_t len)
 {
 	ssize_t res;

bbs/utils.c

@@ -30,7 +30,6 @@
 #include <sys/time.h> /* use gettimeofday */
 #include <uuid/uuid.h> /* use uuid_generate, uuid_unparse */
 #include <syscall.h>
-#include <sys/sendfile.h>
 #include <libgen.h> /* use dirname */
 
 #include "include/utils.h"
@@ -868,7 +867,7 @@ int bbs_copy_file(int srcfd, int destfd, int start, int bytes)
 	if (copied == -1 && errno == ENOSYS) {
 		/* Okay, the actual syscall doesn't even exist. Fall back to sendfile. */
 		fellback = 1;
-		copied = (int) sendfile(destfd, srcfd, &offset, (size_t) bytes);
+		copied = (int) bbs_sendfile(destfd, srcfd, &offset, (size_t) bytes);
 	}
 	if (copied == -1) {
 		bbs_error("%s %d -> %d failed: %s\n", fellback ? "sendfile" : "copy_file_range", srcfd, destfd, strerror(errno));
@@ -894,7 +893,7 @@ ssize_t bbs_send_file(const char *filepath, int wfd)
 	size = lseek(fd, 0, SEEK_END);
 	lseek(fd, 0, SEEK_SET);
 	offset = 0;
-	sent = sendfile(wfd, fd, &offset, (size_t) size);
+	sent = bbs_sendfile(wfd, fd, &offset, (size_t) size);
 	close(fd);
 	if (sent != size) {
 		bbs_error("Wanted to write %lu bytes but only wrote %ld?\n", size, sent);

include/node.h

@@ -502,6 +502,19 @@ ssize_t bbs_write(int fd, const char *buf, size_t len);
  */
 ssize_t bbs_timed_write(int fd, const char *buf, size_t len, int ms);
 
+/*!
+ * \brief Wrapper around sendfile(2) that attempts to fully copy the requested number of bytes
+ * \param out_fd File descriptor open for writing. May be any file (kernels >= 2.6.33)
+ * \param in_fd File descriptor open for reading. Must support mmap(2)-like operations (i.e. cannot be a socket).
+ *               If offset is NULL, offset of in_fd is adjusted to reflect number of bytes read.
+ * \param offset If non-NULL, offset from which to begin reading; upon return, will be the offset of the first unread byte.
+ *               If NULL, read starting at the offset of in_fd and update the file's offset.
+ * \param count Number of bytes to copy from in_fd to out_fd.
+ * \return Number of bytes written to out_fd
+ * \retval -1 on failure
+ */
+ssize_t bbs_sendfile(int out_fd, int in_fd, off_t *offset, size_t count);
+
 /*!
  * \brief Write to a file descriptor associated with a node (but not necessarily the node file descriptor)
  * \param node Node associated with this file descriptor

modules/mod_http.c

@@ -34,7 +34,6 @@
 #include <sys/socket.h>
 #include <pthread.h>
 #include <signal.h>
-#include <sys/sendfile.h>
 #include <magic.h>
 #include <sys/wait.h>
 
@@ -2267,10 +2266,9 @@ enum http_response_code http_static(struct http_session *http, const char *filen
 	if (ranges) {
 		if (rangeparts == 1) {
 			offset = a;
-			written = sendfile(http->wfd, fd, &offset, rangebytes);
+			written = bbs_sendfile(http->wfd, fd, &offset, rangebytes);
 			close(fd);
 			if (written != (ssize_t) rangebytes) {
-				bbs_error("sendfile failed (copied %lu bytes instead of %lu)\n", written, rangebytes);
 				http->req->keepalive = 0;
 			}
 			http->res->sentbytes += (size_t) rangebytes;
@@ -2285,9 +2283,8 @@ enum http_response_code http_static(struct http_session *http, const char *filen
 				http_writef(http, "\r\n");
 				offset = a;
 				bbs_debug(5, "Sending %ld-byte range beginning at offset %lu\n", thisrangebytes, offset);
-				written = sendfile(http->wfd, fd, &offset, (size_t) thisrangebytes);
+				written = bbs_sendfile(http->wfd, fd, &offset, (size_t) thisrangebytes);
 				if (written != (ssize_t) thisrangebytes) {
-					bbs_error("sendfile failed (copied %lu bytes instead of %lu)\n", written, thisrangebytes);
 					close(fd);
 					http->req->keepalive = 0;
 					return http->res->code;
@@ -2299,10 +2296,9 @@ enum http_response_code http_static(struct http_session *http, const char *filen
 			http_writef(http, "--%s--", RANGE_SEPARATOR); /* Final multipart boundary */
 		}
 	} else {
-		written = sendfile(http->wfd, fd, &offset, (size_t) st->st_size);
+		written = bbs_sendfile(http->wfd, fd, &offset, (size_t) st->st_size);
 		close(fd);
 		if (written != (ssize_t) st->st_size) {
-			bbs_error("sendfile failed (copied %lu bytes instead of %lu)\n", written, st->st_size);
 			http->req->keepalive = 0;
 		}
 		http->res->sentbytes += (size_t) st->st_size;

modules/mod_smtp_delivery_external.c

@@ -23,7 +23,6 @@
 
 #include <ctype.h>
 #include <time.h>
-#include <sys/sendfile.h>
 
 #include <arpa/inet.h>
 #include <netdb.h>
@@ -250,6 +249,9 @@ static void process_capabilities(int *restrict caps, int *restrict maxsendsize,
 		}
 	} else if (!strcasecmp(capname, "CHUNKING") || !strcasecmp(capname, "SMTPUTF8") || !strcasecmp(capname, "VRFY") || !strcasecmp(capname, "ETRN") || !strcasecmp(capname, "DSN") || !strcasecmp(capname, "HELP")) {
 		/* Don't care about */
+	} else if (!strcmp(capname, "PIPECONNECT")) {
+		/* Don't care about, at the moment, but could be used in the future to optimize:
+		 * https://www.exim.org/exim-html-current/doc/html/spec_html/ch-main_configuration.html */
 	} else if (!strcmp(capname, "AUTH=LOGIN PLAIN")) {
 		/* Ignore: this SMTP server advertises this capability (even though it's malformed) to support some broken clients */
 	} else {
@@ -536,13 +538,12 @@ static int try_send(struct smtp_session *smtp, struct smtp_tx_data *tx, const ch
 	}
 
 	/* sendfile will be much more efficient than reading the file ourself, as email body could be quite large, and we don't need to involve userspace. */
-	res = (int) sendfile(client.wfd, datafd, &send_offset, writelen);
+	res = (int) bbs_sendfile(client.wfd, datafd, &send_offset, writelen);
 
 	/* XXX If email doesn't end in CR LF, we need to tack that on. But ONLY if it doesn't already end in CR LF. */
 	smtp_client_send(&client, "\r\n.\r\n"); /* (end of) EOM */
 	tx->stage = "end of DATA";
 	if (res != (int) writelen) { /* Failed to write full message */
-		bbs_error("Wanted to write %lu bytes but wrote only %d?\n", writelen, res);
 		res = -1;
 		goto cleanup;
 	}

modules/mod_smtp_delivery_local.c

@@ -20,7 +20,6 @@
 #include "include/bbs.h"
 
 #include <time.h>
-#include <sys/sendfile.h>
 
 #include "include/module.h"
 #include "include/config.h"
@@ -294,7 +293,7 @@ static int upload_file(struct smtp_session *smtp, struct smtp_msg_process *mproc
 		IMAP_CLIENT_EXPECT(&client, "+");
 	}
 
-	res = sendfile(client.wfd, srcfd, &offset, datalen); /* Don't use bbs_copy_file, the target is a pipe/socket, not a file */
+	res = bbs_sendfile(client.wfd, srcfd, &offset, datalen); /* Don't use bbs_copy_file, the target is a pipe/socket, not a file */
 	if (res != (ssize_t) datalen) {
 		bbs_warning("Wanted to upload %lu bytes but only uploaded %ld? (%s)\n", datalen, res, strerror(errno));
 		goto cleanup;

nets/net_ftp.c

@@ -26,7 +26,6 @@
 #include <string.h>
 #include <unistd.h>
 #include <pthread.h>
-#include <sys/sendfile.h>
 #include <dirent.h>
 
 #include "include/module.h"
@@ -551,7 +550,7 @@ static void *ftp_handler(void *varg)
 				if (DATA_INIT()) {
 					break;
 				}
-				res = (int) sendfile(ftp->wfd2, fileno(fp), NULL, (size_t) filestat.st_size); /* More convenient and efficient than manually relaying using read/write */
+				res = (int) bbs_sendfile(ftp->wfd2, fileno(fp), NULL, (size_t) filestat.st_size); /* More convenient and efficient than manually relaying using read/write */
 				DATA_DONE(fp, pasv_fd);
 				if (res != filestat.st_size) {
 					bbs_error("File transfer failed: %s\n", strerror(errno));

nets/net_gopher.c

@@ -24,7 +24,6 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <signal.h>
-#include <sys/sendfile.h>
 
 #include "include/module.h"
 #include "include/config.h"
@@ -107,10 +106,7 @@ static void *gopher_handler(void *varg)
 			size = (int) ftell(fp);
 			rewind(fp); /* Be kind, rewind */
 
-			res = (int) sendfile(node->fd, fileno(fp), &offset, (size_t) size); /* We must manually tell it the offset or it will be at the EOF, even with rewind() */
-			if (res != size) {
-				bbs_error("sendfile failed (%ld): %s\n", res, strerror(errno));
-			}
+			res = (int) bbs_sendfile(node->fd, fileno(fp), &offset, (size_t) size); /* We must manually tell it the offset or it will be at the EOF, even with rewind() */
 			fclose(fp);
 		} else {
 			bbs_error("fopen failed: %s\n", strerror(errno));

nets/net_imap.c

@@ -126,7 +126,6 @@
 #include <dirent.h>
 #include <poll.h>
 #include <utime.h>
-#include <sys/sendfile.h>
 
 #include "include/tls.h"
 

nets/net_imap/imap_server_fetch.c

@@ -20,7 +20,6 @@
 #include "include/bbs.h"
 
 #include <ctype.h>
-#include <sys/sendfile.h>
 #include <dirent.h>
 
 #include "include/node.h"
@@ -488,14 +487,12 @@ static int process_fetch_finalize(struct imap_session *imap, struct fetch_reques
 			imap_send(imap, "%d FETCH (%s%s%s %s {%ld}", seqno, S_IF(dyn), dyn ? " " : "", response, resptype, size); /* No close paren here, last write will do that */
 
 			pthread_mutex_lock(&imap->lock);
-			res = sendfile(imap->wfd, fileno(fp), &offset, (size_t) size); /* We must manually tell it the offset or it will be at the EOF, even with rewind() */
+			res = bbs_sendfile(imap->wfd, fileno(fp), &offset, (size_t) size); /* We must manually tell it the offset or it will be at the EOF, even with rewind() */
 			bbs_node_fd_writef(imap->node, imap->wfd, ")\r\n"); /* And the finale (don't use imap_send for this) */
 			pthread_mutex_unlock(&imap->lock);
 
 			fclose(fp);
-			if (res != (ssize_t) size) {
-				bbs_error("sendfile(%s) failed (%ld != %ld): %s\n", fullname, res, size, strerror(errno));
-			} else {
+			if (res == (ssize_t) size) {
 				imap_debug(5, "Sent %ld/%ld-byte body for %s\n", res, fullsize, fullname); /* either partial or entire body */
 			}
 		} else {

nets/net_pop3.c

@@ -27,7 +27,6 @@
 #include <ctype.h>
 #include <signal.h>
 #include <unistd.h>
-#include <sys/sendfile.h>
 #include <dirent.h>
 
 #include "include/tls.h"
@@ -496,10 +495,7 @@ static int on_retr(const char *dir_name, const char *filename, struct pop3_sessi
 
 	pop3_ok(pop3, "%u octets", realsize);
 	offset = 0;
-	res = (unsigned int) sendfile(pop3->wfd, fileno(fp), &offset, realsize);
-	if (res != realsize) {
-		bbs_error("Wanted to send %d bytes but only sent %d?\n", realsize, res);
-	}
+	res = (unsigned int) bbs_sendfile(pop3->wfd, fileno(fp), &offset, realsize);
 	bbs_debug(6, "Sent %d bytes\n", res);
 	bbs_node_fd_writef(pop3->node, pop3->wfd, ".\r\n");
 	fclose(fp);

nets/net_smtp.c

@@ -43,7 +43,6 @@
 #include <ctype.h>
 #include <signal.h>
 #include <unistd.h>
-#include <sys/sendfile.h>
 #include <sys/ioctl.h>
 
 #include <dirent.h> /* for msg_to_filename */
@@ -345,7 +344,6 @@ static int smtp_tarpit(struct smtp_session *smtp, int code, const char *message)
 static int fcrdns_check(struct smtp_session *smtp)
 {
 	char hostname[256];
-	char ip[256];
 
 	/* This is a relatively lenient check that most any legitimate SMTP server should pass.
 	 * The hostname provided by the MTA must resolve to the IP address of the connection,
@@ -369,7 +367,7 @@ static int fcrdns_check(struct smtp_session *smtp)
 		bbs_warning("Unable to look up reverse DNS record for %s\n", smtp->node->ip);
 		smtp->failures += 4; /* Heavy penalty */
 	} else if (!bbs_hostname_has_ip(hostname, smtp->node->ip)) { /* Ensure that there's a match, with at least one A record */
-		bbs_warning("FCrDNS check failed: %s != %s\n", ip, smtp->node->ip);
+		bbs_warning("FCrDNS check failed: %s != %s\n", hostname, smtp->node->ip);
 		smtp->failures += 5;
 	}
 	return 0;
@@ -435,6 +433,10 @@ static int smtp_ip_mismatch(const char *actual, const char *hostname)
 	 * If it's just a raw IP address, that is not valid.
 	 * IPv6 literals as described in RFC 5321 4.1.3 are not supported. */
 	if (bbs_hostname_is_ipv4(hostname)) {
+		if (!strcmp(actual, hostname)) {
+			bbs_warning("SMTP IP address '%s' is in non-canonical format\n", hostname); /* Should be surrounded by [] */
+			return 0;
+		}
 		return -1;
 	} else if (*hostname == '[' && *(hostname + 1)) {
 		/* Domain literal */
@@ -1680,6 +1682,10 @@ static int expand_and_deliver(struct smtp_session *smtp, const char *filename, s
 			continue;
 		}
 
+		if (*recipient != '<') {
+			bbs_warning("Malformed recipient: %s\n", recipient);
+		}
+
 		dup = strdup(recipient);
 		if (ALLOC_FAILURE(dup)) {
 			goto next;