The branch stable/13 has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=2af3758a3294f7f1cbbeeb0d62e2ce62917f1103
commit 2af3758a3294f7f1cbbeeb0d62e2ce62917f1103 Author: Alan Somers <[email protected]> AuthorDate: 2022-01-03 00:51:44 +0000 Commit: Alan Somers <[email protected]> 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 <[email protected]> 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 <[email protected]> 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);
