git: 2af3758a3294 - stable/13 - Various fixes for ggatec and ggated
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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);