git: 2af3758a3294 - stable/13 - Various fixes for ggatec and ggated

From: Alan Somers <asomers_at_FreeBSD.org>
Date: Thu, 17 Feb 2022 04:07:44 UTC
The branch stable/13 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=2af3758a3294f7f1cbbeeb0d62e2ce62917f1103

commit 2af3758a3294f7f1cbbeeb0d62e2ce62917f1103
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-01-03 00:51:44 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-02-17 04:07:15 +0000

    Various fixes for ggatec and ggated
    
    Dynamically size buffers in ggatec. Instead of static size on the stack.
    Add flush support.
    
    Submitted by:   Johannes Totz <jo@bruelltuete.com>
    Reviewed by:    asomers
    Differential Revision: https://reviews.freebsd.org/D31722
    
    (cherry picked from commit 6226477a462f5ffbeacafdc9461524c95a7eb154)
    
    geom_gate: ensure readprov is null-terminated
    
    With crafted input to the G_GATE_CMD_CREATE ioctl, geom_gate can be made
    to print kernel memory to the system console, potentially revealing
    sensitive data from whatever was previously in that memory page.
    
    But but but: this is a case of the sys admin misconfiguring, and you'd
    need root privileges to do this.
    
    Submitted By:   Johannes Totz <jo@bruelltuete.com>
    Reviewed By:    asomers
    Differential Revision: https://reviews.freebsd.org/D31727
    
    (cherry picked from commit f284bed200b04e48c4ae87a50f4a8a957b0a10ad)
---
 sbin/ggate/ggatec/ggatec.c | 112 ++++++++++++++++++++++++++++++---------------
 sbin/ggate/ggated/ggated.c |  14 +++++-
 sbin/ggate/shared/ggate.h  |   1 +
 sys/geom/gate/g_gate.c     |  42 +++++++++++------
 4 files changed, 117 insertions(+), 52 deletions(-)

diff --git a/sbin/ggate/ggatec/ggatec.c b/sbin/ggate/ggatec/ggatec.c
index 0695dae0dca2..dfd9506e5e26 100644
--- a/sbin/ggate/ggatec/ggatec.c
+++ b/sbin/ggate/ggatec/ggatec.c
@@ -74,6 +74,7 @@ static int sendfd, recvfd;
 static uint32_t token;
 static pthread_t sendtd, recvtd;
 static int reconnect;
+static int initialbuffersize = 131072;
 
 static void
 usage(void)
@@ -94,18 +95,25 @@ send_thread(void *arg __unused)
 {
 	struct g_gate_ctl_io ggio;
 	struct g_gate_hdr hdr;
-	char buf[MAXPHYS];
-	ssize_t data;
+	size_t buf_capacity;
+	ssize_t numbytesprocd;
 	int error;
+	char *newbuf;
 
 	g_gate_log(LOG_NOTICE, "%s: started!", __func__);
 
+	buf_capacity = initialbuffersize;
+
 	ggio.gctl_version = G_GATE_VERSION;
 	ggio.gctl_unit = unit;
-	ggio.gctl_data = buf;
+	ggio.gctl_data = malloc(buf_capacity);
+	if (ggio.gctl_data == NULL) {
+		g_gate_log(LOG_ERR, "%s: Cannot alloc buffer.", __func__);
+		pthread_exit(NULL);
+	}
 
 	for (;;) {
-		ggio.gctl_length = sizeof(buf);
+		ggio.gctl_length = buf_capacity;
 		ggio.gctl_error = 0;
 		g_gate_ioctl(G_GATE_CMD_START, &ggio);
 		error = ggio.gctl_error;
@@ -118,17 +126,22 @@ send_thread(void *arg __unused)
 			/* Exit gracefully. */
 			g_gate_close_device();
 			exit(EXIT_SUCCESS);
-#if 0
+
 		case ENOMEM:
+		{
 			/* Buffer too small. */
-			ggio.gctl_data = realloc(ggio.gctl_data,
+			g_gate_log(LOG_DEBUG, "buffer too small. new size: %u",
 			    ggio.gctl_length);
-			if (ggio.gctl_data != NULL) {
-				bsize = ggio.gctl_length;
-				goto once_again;
+			newbuf = malloc(ggio.gctl_length);
+			if (newbuf != NULL) {
+				free(ggio.gctl_data);
+				ggio.gctl_data = newbuf;
+				buf_capacity = ggio.gctl_length;
+				continue;
 			}
 			/* FALLTHROUGH */
-#endif
+		}
+
 		case ENXIO:
 		default:
 			g_gate_xlog("ioctl(/dev/%s): %s.", G_GATE_CTL_NAME,
@@ -145,16 +158,12 @@ send_thread(void *arg __unused)
 		case BIO_WRITE:
 			hdr.gh_cmd = GGATE_CMD_WRITE;
 			break;
+		case BIO_FLUSH:
+			hdr.gh_cmd = GGATE_CMD_FLUSH;
+			break;
 		default:
-			g_gate_log(LOG_NOTICE, "Unknown gctl_cmd: %i", ggio.gctl_cmd);
-			ggio.gctl_error = EOPNOTSUPP;
-			g_gate_ioctl(G_GATE_CMD_DONE, &ggio);
-			continue;
-		}
-
-		/* Don't send requests for more data than we can handle the response for! */
-		if (ggio.gctl_length > MAXPHYS) {
-			g_gate_log(LOG_ERR, "Request too big: %zd", ggio.gctl_length);
+			g_gate_log(LOG_NOTICE, "Unknown gctl_cmd: %i",
+			    ggio.gctl_cmd);
 			ggio.gctl_error = EOPNOTSUPP;
 			g_gate_ioctl(G_GATE_CMD_DONE, &ggio);
 			continue;
@@ -166,12 +175,12 @@ send_thread(void *arg __unused)
 		hdr.gh_error = 0;
 		g_gate_swap2n_hdr(&hdr);
 
-		data = g_gate_send(sendfd, &hdr, sizeof(hdr), MSG_NOSIGNAL);
+		numbytesprocd = g_gate_send(sendfd, &hdr, sizeof(hdr), MSG_NOSIGNAL);
 		g_gate_log(LOG_DEBUG, "Sent hdr packet.");
 		g_gate_swap2h_hdr(&hdr);
 		if (reconnect)
 			break;
-		if (data != sizeof(hdr)) {
+		if (numbytesprocd != sizeof(hdr)) {
 			g_gate_log(LOG_ERR, "Lost connection 1.");
 			reconnect = 1;
 			pthread_kill(recvtd, SIGUSR1);
@@ -179,18 +188,19 @@ send_thread(void *arg __unused)
 		}
 
 		if (hdr.gh_cmd == GGATE_CMD_WRITE) {
-			data = g_gate_send(sendfd, ggio.gctl_data,
+			numbytesprocd = g_gate_send(sendfd, ggio.gctl_data,
 			    ggio.gctl_length, MSG_NOSIGNAL);
 			if (reconnect)
 				break;
-			if (data != ggio.gctl_length) {
-				g_gate_log(LOG_ERR, "Lost connection 2 (%zd != %zd).", data, (ssize_t)ggio.gctl_length);
+			if (numbytesprocd != ggio.gctl_length) {
+				g_gate_log(LOG_ERR, "Lost connection 2 (%zd != %zd).",
+				    numbytesprocd, (ssize_t)ggio.gctl_length);
 				reconnect = 1;
 				pthread_kill(recvtd, SIGUSR1);
 				break;
 			}
 			g_gate_log(LOG_DEBUG, "Sent %zd bytes (offset=%"
-			    PRIu64 ", length=%" PRIu32 ").", data,
+			    PRIu64 ", length=%" PRIu32 ").", numbytesprocd,
 			    hdr.gh_offset, hdr.gh_length);
 		}
 	}
@@ -203,22 +213,29 @@ recv_thread(void *arg __unused)
 {
 	struct g_gate_ctl_io ggio;
 	struct g_gate_hdr hdr;
-	char buf[MAXPHYS];
-	ssize_t data;
+	ssize_t buf_capacity;
+	ssize_t numbytesprocd;
+	char *newbuf;
 
 	g_gate_log(LOG_NOTICE, "%s: started!", __func__);
 
+	buf_capacity = initialbuffersize;
+
 	ggio.gctl_version = G_GATE_VERSION;
 	ggio.gctl_unit = unit;
-	ggio.gctl_data = buf;
+	ggio.gctl_data = malloc(buf_capacity);
+	if (ggio.gctl_data == NULL) {
+		g_gate_log(LOG_ERR, "%s: Cannot alloc buffer.", __func__);
+		pthread_exit(NULL);
+	}
 
 	for (;;) {
-		data = g_gate_recv(recvfd, &hdr, sizeof(hdr), MSG_WAITALL);
+		numbytesprocd = g_gate_recv(recvfd, &hdr, sizeof(hdr), MSG_WAITALL);
 		if (reconnect)
 			break;
 		g_gate_swap2h_hdr(&hdr);
-		if (data != sizeof(hdr)) {
-			if (data == -1 && errno == EAGAIN)
+		if (numbytesprocd != sizeof(hdr)) {
+			if (numbytesprocd == -1 && errno == EAGAIN)
 				continue;
 			g_gate_log(LOG_ERR, "Lost connection 3.");
 			reconnect = 1;
@@ -233,26 +250,33 @@ recv_thread(void *arg __unused)
 		ggio.gctl_length = hdr.gh_length;
 		ggio.gctl_error = hdr.gh_error;
 
-		/* Do not overflow our buffer if there is a bogus response. */
-		if (ggio.gctl_length > (off_t) sizeof(buf)) {
-			g_gate_log(LOG_ERR, "Received too big response: %zd", ggio.gctl_length);
-			break;
+		if (ggio.gctl_length > buf_capacity) {
+			newbuf = malloc(ggio.gctl_length);
+			if (newbuf != NULL) {
+				free(ggio.gctl_data);
+				ggio.gctl_data = newbuf;
+				buf_capacity = ggio.gctl_length;
+			} else {
+				g_gate_log(LOG_ERR, "Received too big response: %zd",
+				    ggio.gctl_length);
+				break;
+			}
 		}
 
 		if (ggio.gctl_error == 0 && ggio.gctl_cmd == GGATE_CMD_READ) {
-			data = g_gate_recv(recvfd, ggio.gctl_data,
+			numbytesprocd = g_gate_recv(recvfd, ggio.gctl_data,
 			    ggio.gctl_length, MSG_WAITALL);
 			if (reconnect)
 				break;
 			g_gate_log(LOG_DEBUG, "Received data packet.");
-			if (data != ggio.gctl_length) {
+			if (numbytesprocd != ggio.gctl_length) {
 				g_gate_log(LOG_ERR, "Lost connection 4.");
 				reconnect = 1;
 				pthread_kill(sendtd, SIGUSR1);
 				break;
 			}
 			g_gate_log(LOG_DEBUG, "Received %d bytes (offset=%"
-			    PRIu64 ", length=%" PRIu32 ").", data,
+			    PRIu64 ", length=%" PRIu32 ").", numbytesprocd,
 			    hdr.gh_offset, hdr.gh_length);
 		}
 
@@ -509,6 +533,16 @@ g_gatec_rescue(void)
 	g_gatec_loop();
 }
 
+static void
+init_initial_buffer_size()
+{
+	int value;
+	size_t intsize;
+	intsize = sizeof(initialbuffersize);
+	if (sysctlbyname("kern.maxphys", &value, &intsize, NULL, 0) == 0)
+		initialbuffersize = value;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -624,6 +658,8 @@ main(int argc, char *argv[])
 	argc -= optind;
 	argv += optind;
 
+	init_initial_buffer_size();
+
 	switch (action) {
 	case CREATE:
 		if (argc != 2)
diff --git a/sbin/ggate/ggated/ggated.c b/sbin/ggate/ggated/ggated.c
index 226ba1ce72de..7cacbf58037e 100644
--- a/sbin/ggate/ggated/ggated.c
+++ b/sbin/ggate/ggated/ggated.c
@@ -726,7 +726,6 @@ disk_thread(void *arg)
 		/*
 		 * Check the request.
 		 */
-		assert(req->r_cmd == GGATE_CMD_READ || req->r_cmd == GGATE_CMD_WRITE);
 		assert(req->r_offset + req->r_length <= (uintmax_t)conn->c_mediasize);
 		assert((req->r_offset % conn->c_sectorsize) == 0);
 		assert((req->r_length % conn->c_sectorsize) == 0);
@@ -750,6 +749,19 @@ disk_thread(void *arg)
 			free(req->r_data);
 			req->r_data = NULL;
 			break;
+		case GGATE_CMD_FLUSH:
+			data = fsync(fd);
+			if (data != 0)
+				req->r_error = errno;
+			break;
+		default:
+			g_gate_log(LOG_DEBUG, "Unsupported request: %i", req->r_cmd);
+			req->r_error = EOPNOTSUPP;
+			if (req->r_data != NULL) {
+				free(req->r_data);
+				req->r_data = NULL;
+			}
+			break;
 		}
 		if (data != (ssize_t)req->r_length) {
 			/* Report short reads/writes as I/O errors. */
diff --git a/sbin/ggate/shared/ggate.h b/sbin/ggate/shared/ggate.h
index e2e1a57d817c..d399b247cd75 100644
--- a/sbin/ggate/shared/ggate.h
+++ b/sbin/ggate/shared/ggate.h
@@ -57,6 +57,7 @@
 
 #define	GGATE_CMD_READ		0
 #define	GGATE_CMD_WRITE		1
+#define	GGATE_CMD_FLUSH		3
 
 extern int g_gate_devfd;
 extern int g_gate_verbose;
diff --git a/sys/geom/gate/g_gate.c b/sys/geom/gate/g_gate.c
index c8f6f4a1b3b7..14ec0cc2e9d2 100644
--- a/sys/geom/gate/g_gate.c
+++ b/sys/geom/gate/g_gate.c
@@ -468,7 +468,8 @@ g_gate_create(struct g_gate_ctl_create *ggio)
 	struct g_geom *gp;
 	struct g_provider *pp, *ropp;
 	struct g_consumer *cp;
-	char name[NAME_MAX];
+	char name[NAME_MAX + 1];
+	char readprov[NAME_MAX + 1];
 	int error = 0, unit;
 
 	if (ggio->gctl_mediasize <= 0) {
@@ -506,7 +507,9 @@ g_gate_create(struct g_gate_ctl_create *ggio)
 
 	sc = malloc(sizeof(*sc), M_GATE, M_WAITOK | M_ZERO);
 	sc->sc_flags = (ggio->gctl_flags & G_GATE_USERFLAGS);
-	strlcpy(sc->sc_info, ggio->gctl_info, sizeof(sc->sc_info));
+	memset(sc->sc_info, 0, sizeof(sc->sc_info));
+	strncpy(sc->sc_info, ggio->gctl_info,
+	    MIN(sizeof(sc->sc_info) - 1, sizeof(ggio->gctl_info)));
 	sc->sc_seq = 1;
 	bioq_init(&sc->sc_inqueue);
 	bioq_init(&sc->sc_outqueue);
@@ -523,9 +526,11 @@ g_gate_create(struct g_gate_ctl_create *ggio)
 	sc->sc_unit = g_gate_getunit(ggio->gctl_unit, &error);
 	if (sc->sc_unit < 0)
 		goto fail1;
-	if (ggio->gctl_unit == G_GATE_NAME_GIVEN)
-		snprintf(name, sizeof(name), "%s", ggio->gctl_name);
-	else {
+	if (ggio->gctl_unit == G_GATE_NAME_GIVEN) {
+		memset(name, 0, sizeof(name));
+		strncpy(name, ggio->gctl_name,
+		    MIN(sizeof(name) - 1, sizeof(ggio->gctl_name)));
+	} else {
 		snprintf(name, sizeof(name), "%s%d", G_GATE_PROVIDER_NAME,
 		    sc->sc_unit);
 	}
@@ -538,6 +543,8 @@ g_gate_create(struct g_gate_ctl_create *ggio)
 		error = EEXIST;
 		goto fail1;
 	}
+	// local stack buffer 'name' assigned here temporarily only.
+	// the real provider name is assigned below.
 	sc->sc_name = name;
 	g_gate_units[sc->sc_unit] = sc;
 	g_gate_nunits++;
@@ -548,10 +555,12 @@ g_gate_create(struct g_gate_ctl_create *ggio)
 	if (ggio->gctl_readprov[0] == '\0') {
 		ropp = NULL;
 	} else {
-		ropp = g_provider_by_name(ggio->gctl_readprov);
+		memset(readprov, 0, sizeof(readprov));
+		strncpy(readprov, ggio->gctl_readprov,
+		    MIN(sizeof(readprov) - 1, sizeof(ggio->gctl_readprov)));
+		ropp = g_provider_by_name(readprov);
 		if (ropp == NULL) {
-			G_GATE_DEBUG(1, "Provider %s doesn't exist.",
-			    ggio->gctl_readprov);
+			G_GATE_DEBUG(1, "Provider %s doesn't exist.", readprov);
 			error = EINVAL;
 			goto fail2;
 		}
@@ -633,6 +642,7 @@ fail1:
 static int
 g_gate_modify(struct g_gate_softc *sc, struct g_gate_ctl_modify *ggio)
 {
+	char readprov[NAME_MAX + 1];
 	struct g_provider *pp;
 	struct g_consumer *cp;
 	int done, error;
@@ -651,9 +661,11 @@ g_gate_modify(struct g_gate_softc *sc, struct g_gate_ctl_modify *ggio)
 		return (0);
 	}
 
-	if ((ggio->gctl_modify & GG_MODIFY_INFO) != 0)
-		(void)strlcpy(sc->sc_info, ggio->gctl_info, sizeof(sc->sc_info));
-
+	if ((ggio->gctl_modify & GG_MODIFY_INFO) != 0) {
+		memset(sc->sc_info, 0, sizeof(sc->sc_info));
+		strncpy(sc->sc_info, ggio->gctl_info,
+		    MIN(sizeof(sc->sc_info) - 1, sizeof(ggio->gctl_info)));
+	}
 	cp = NULL;
 
 	if ((ggio->gctl_modify & GG_MODIFY_READPROV) != 0) {
@@ -668,11 +680,15 @@ g_gate_modify(struct g_gate_softc *sc, struct g_gate_ctl_modify *ggio)
 		} else
 			mtx_unlock(&sc->sc_read_mtx);
 		if (ggio->gctl_readprov[0] != '\0') {
-			pp = g_provider_by_name(ggio->gctl_readprov);
+			memset(readprov, 0, sizeof(readprov));
+			strncpy(readprov, ggio->gctl_readprov,
+			    MIN(sizeof(readprov) - 1,
+			    sizeof(ggio->gctl_readprov)));
+			pp = g_provider_by_name(readprov);
 			if (pp == NULL) {
 				g_topology_unlock();
 				G_GATE_DEBUG(1, "Provider %s doesn't exist.",
-				    ggio->gctl_readprov);
+				    readprov);
 				return (EINVAL);
 			}
 			cp = g_new_consumer(sc->sc_provider->geom);