git: f284bed200b0 - main - geom_gate: ensure readprov is null-terminated
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 03 Jan 2022 01:33:56 UTC
The branch main has been updated by asomers:
URL: https://cgit.FreeBSD.org/src/commit/?id=f284bed200b04e48c4ae87a50f4a8a957b0a10ad
commit f284bed200b04e48c4ae87a50f4a8a957b0a10ad
Author: Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-01-03 01:00:30 +0000
Commit: Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-01-03 01:01:23 +0000
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>
MFC after: 2 weeks
Reviewed By: asomers
Differential Revision: https://reviews.freebsd.org/D31727
---
sys/geom/gate/g_gate.c | 42 +++++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 13 deletions(-)
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);