From f5b4099e6b72fd7ac9f620c389229a9a4bf09992 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Sun, 4 Feb 2018 14:49:55 +0000 Subject: [PATCH] geom: don't write stack garbage in disk labels Most consumers of g_metadata_store were passing in partially unallocated memory, resulting in stack garbage being written to disk labels. Fix them by zeroing the memory first. gvirstor repeated the same mistake, but in the kernel. Also, glabel's label contained a fixed-size string that wasn't initialized to zero. PR: 222077 Reported by: Maxim Khitrov Reviewed by: cem MFC after: 3 weeks X-MFC-With: 323314 X-MFC-With: 323338 Differential Revision: https://reviews.freebsd.org/D14164 --- sbin/geom/class/cache/geom_cache.c | 1 + sbin/geom/class/concat/geom_concat.c | 1 + sbin/geom/class/journal/geom_journal.c | 1 + sbin/geom/class/label/geom_label.c | 2 ++ sbin/geom/class/mirror/geom_mirror.c | 1 + sbin/geom/class/raid3/geom_raid3.c | 1 + sbin/geom/class/shsec/geom_shsec.c | 1 + sbin/geom/class/stripe/geom_stripe.c | 1 + sbin/geom/misc/subr.c | 7 +++++++ sys/geom/virstor/g_virstor.c | 1 + 10 files changed, 17 insertions(+) diff --git a/sbin/geom/class/cache/geom_cache.c b/sbin/geom/class/cache/geom_cache.c index fca9f4d7ca37..4e76da2ce7a8 100644 --- a/sbin/geom/class/cache/geom_cache.c +++ b/sbin/geom/class/cache/geom_cache.c @@ -137,6 +137,7 @@ cache_label(struct gctl_req *req) int error, nargs; intmax_t val; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); if (nargs != 2) { gctl_error(req, "Invalid number of arguments."); diff --git a/sbin/geom/class/concat/geom_concat.c b/sbin/geom/class/concat/geom_concat.c index f2a7ffda334a..801bea61cdfd 100644 --- a/sbin/geom/class/concat/geom_concat.c +++ b/sbin/geom/class/concat/geom_concat.c @@ -119,6 +119,7 @@ concat_label(struct gctl_req *req) const char *name; int error, i, hardcode, nargs; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); if (nargs < 2) { gctl_error(req, "Too few arguments."); diff --git a/sbin/geom/class/journal/geom_journal.c b/sbin/geom/class/journal/geom_journal.c index a828e26bfecd..2a174c6e5b1c 100644 --- a/sbin/geom/class/journal/geom_journal.c +++ b/sbin/geom/class/journal/geom_journal.c @@ -144,6 +144,7 @@ journal_label(struct gctl_req *req) intmax_t jsize, msize, ssize; int error, force, i, nargs, checksum, hardcode; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); str = NULL; /* gcc */ diff --git a/sbin/geom/class/label/geom_label.c b/sbin/geom/class/label/geom_label.c index e69b93fdf8f9..f51e87ecb57d 100644 --- a/sbin/geom/class/label/geom_label.c +++ b/sbin/geom/class/label/geom_label.c @@ -125,6 +125,7 @@ label_label(struct gctl_req *req) u_char sector[512]; int error, nargs; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); if (nargs != 2) { gctl_error(req, "Invalid number of arguments."); @@ -145,6 +146,7 @@ label_label(struct gctl_req *req) strlcpy(md.md_magic, G_LABEL_MAGIC, sizeof(md.md_magic)); md.md_version = G_LABEL_VERSION; label = gctl_get_ascii(req, "arg0"); + bzero(md.md_label, sizeof(md.md_label)); strlcpy(md.md_label, label, sizeof(md.md_label)); md.md_provsize = g_get_mediasize(name); if (md.md_provsize == 0) { diff --git a/sbin/geom/class/mirror/geom_mirror.c b/sbin/geom/class/mirror/geom_mirror.c index 668b0a399548..a1b399338814 100644 --- a/sbin/geom/class/mirror/geom_mirror.c +++ b/sbin/geom/class/mirror/geom_mirror.c @@ -188,6 +188,7 @@ mirror_label(struct gctl_req *req) intmax_t val; int error, i, nargs, bal, hardcode; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); if (nargs < 2) { gctl_error(req, "Too few arguments."); diff --git a/sbin/geom/class/raid3/geom_raid3.c b/sbin/geom/class/raid3/geom_raid3.c index 1ad347dae384..17d3187d5cf4 100644 --- a/sbin/geom/class/raid3/geom_raid3.c +++ b/sbin/geom/class/raid3/geom_raid3.c @@ -151,6 +151,7 @@ raid3_label(struct gctl_req *req) int hardcode, round_robin, verify; int error, i, nargs; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); if (nargs < 4) { gctl_error(req, "Too few arguments."); diff --git a/sbin/geom/class/shsec/geom_shsec.c b/sbin/geom/class/shsec/geom_shsec.c index 78e27b34bff9..308a53b7f9d3 100644 --- a/sbin/geom/class/shsec/geom_shsec.c +++ b/sbin/geom/class/shsec/geom_shsec.c @@ -112,6 +112,7 @@ shsec_label(struct gctl_req *req) const char *name; int error, i, nargs, hardcode; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); if (nargs <= 2) { gctl_error(req, "Too few arguments."); diff --git a/sbin/geom/class/stripe/geom_stripe.c b/sbin/geom/class/stripe/geom_stripe.c index 038d2ce310a0..175ddada2e33 100644 --- a/sbin/geom/class/stripe/geom_stripe.c +++ b/sbin/geom/class/stripe/geom_stripe.c @@ -130,6 +130,7 @@ stripe_label(struct gctl_req *req) const char *name; int error, i, nargs, hardcode; + bzero(sector, sizeof(sector)); nargs = gctl_get_int(req, "nargs"); if (nargs < 3) { gctl_error(req, "Too few arguments."); diff --git a/sbin/geom/misc/subr.c b/sbin/geom/misc/subr.c index a8e60f6d286a..3985ae56edc6 100644 --- a/sbin/geom/misc/subr.c +++ b/sbin/geom/misc/subr.c @@ -273,6 +273,13 @@ out: return (error); } +/* + * Actually write the GEOM label to the provider + * + * @param name GEOM provider's name (ie "ada0") + * @param md Pointer to the label data to write + * @param size Size of the data pointed to by md + */ int g_metadata_store(const char *name, const unsigned char *md, size_t size) { diff --git a/sys/geom/virstor/g_virstor.c b/sys/geom/virstor/g_virstor.c index 96791ce7f368..2dd20aa9442e 100644 --- a/sys/geom/virstor/g_virstor.c +++ b/sys/geom/virstor/g_virstor.c @@ -1042,6 +1042,7 @@ write_metadata(struct g_consumer *cp, struct g_virstor_metadata *md) pp = cp->provider; buf = malloc(pp->sectorsize, M_GVIRSTOR, M_WAITOK); + bzero(buf, pp->sectorsize); virstor_metadata_encode(md, buf); g_topology_unlock(); error = g_write_data(cp, pp->mediasize - pp->sectorsize, buf,