From owner-svn-src-stable@freebsd.org Sat Mar 10 04:17:03 2018 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DF238F48AF5; Sat, 10 Mar 2018 04:17:02 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8EF2081B05; Sat, 10 Mar 2018 04:17:02 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 89687B43; Sat, 10 Mar 2018 04:17:02 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w2A4H2Du035819; Sat, 10 Mar 2018 04:17:02 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w2A4H1np035807; Sat, 10 Mar 2018 04:17:01 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201803100417.w2A4H1np035807@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Sat, 10 Mar 2018 04:17:01 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r330737 - in stable/10: sbin/geom/class/cache sbin/geom/class/concat sbin/geom/class/journal sbin/geom/class/label sbin/geom/class/mirror sbin/geom/class/raid3 sbin/geom/class/shsec sbi... X-SVN-Group: stable-10 X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in stable/10: sbin/geom/class/cache sbin/geom/class/concat sbin/geom/class/journal sbin/geom/class/label sbin/geom/class/mirror sbin/geom/class/raid3 sbin/geom/class/shsec sbin/geom/class/stripe sbin/... X-SVN-Commit-Revision: 330737 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 10 Mar 2018 04:17:03 -0000 Author: asomers Date: Sat Mar 10 04:17:01 2018 New Revision: 330737 URL: https://svnweb.freebsd.org/changeset/base/330737 Log: MFC r323314, r323338, r328849 r323314: Audit userspace geom code for leaking memory to disk Any geom class using g_metadata_store, as well as geom_virstor which duplicated g_metadata_store internally, would dump sectorsize - mdsize bytes of userspace memory following the metadata block stored. This is most or all geom classes (gcache, gconcat, geli, gjournal, glabel, gmirror, gmultipath, graid3, gshsec, gstripe, and geom_virstor). PR: 222077 (comment #3) Reported by: Maxim Khitrov Reviewed by: des Security: yes Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D12269 r323338: Fix information leak in geli(8) integrity mode In integrity mode, a larger logical sector (e.g., 4096 bytes) spans several physical sectors (e.g., 512 bytes) on the backing device. Due to hash overhead, a 4096 byte logical sector takes 8.5625 512-byte physical sectors. This means that only 288 bytes (256 data + 32 hash) of the last 512 byte sector are used. The memory allocation used to store the encrypted data to be written to the physical sectors comes from malloc(9) and does not use M_ZERO. Previously, nothing initialized the final physical sector backing each logical sector, aside from the hash + encrypted data portion. So 224 bytes of kernel heap memory was leaked to every block :-(. This patch addresses the issue by initializing the trailing portion of the physical sector in every logical sector to zeros before use. A much simpler but higher overhead fix would be to tag the entire allocation M_ZERO. PR: 222077 Reported by: Maxim Khitrov Reviewed by: emaste Security: yes Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D12272 r328849: 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 X-MFC-With: 323314 X-MFC-With: 323338 Differential Revision: https://reviews.freebsd.org/D14164 Modified: stable/10/sbin/geom/class/cache/geom_cache.c stable/10/sbin/geom/class/concat/geom_concat.c stable/10/sbin/geom/class/journal/geom_journal.c stable/10/sbin/geom/class/label/geom_label.c stable/10/sbin/geom/class/mirror/geom_mirror.c stable/10/sbin/geom/class/raid3/geom_raid3.c stable/10/sbin/geom/class/shsec/geom_shsec.c stable/10/sbin/geom/class/stripe/geom_stripe.c stable/10/sbin/geom/class/virstor/geom_virstor.c stable/10/sbin/geom/misc/subr.c stable/10/sys/geom/eli/g_eli_integrity.c stable/10/sys/geom/virstor/g_virstor.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sbin/geom/class/cache/geom_cache.c ============================================================================== --- stable/10/sbin/geom/class/cache/geom_cache.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/cache/geom_cache.c Sat Mar 10 04:17:01 2018 (r330737) @@ -135,6 +135,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."); Modified: stable/10/sbin/geom/class/concat/geom_concat.c ============================================================================== --- stable/10/sbin/geom/class/concat/geom_concat.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/concat/geom_concat.c Sat Mar 10 04:17:01 2018 (r330737) @@ -117,6 +117,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."); Modified: stable/10/sbin/geom/class/journal/geom_journal.c ============================================================================== --- stable/10/sbin/geom/class/journal/geom_journal.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/journal/geom_journal.c Sat Mar 10 04:17:01 2018 (r330737) @@ -142,6 +142,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 */ Modified: stable/10/sbin/geom/class/label/geom_label.c ============================================================================== --- stable/10/sbin/geom/class/label/geom_label.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/label/geom_label.c Sat Mar 10 04:17:01 2018 (r330737) @@ -117,6 +117,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."); @@ -137,6 +138,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) { Modified: stable/10/sbin/geom/class/mirror/geom_mirror.c ============================================================================== --- stable/10/sbin/geom/class/mirror/geom_mirror.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/mirror/geom_mirror.c Sat Mar 10 04:17:01 2018 (r330737) @@ -176,6 +176,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."); Modified: stable/10/sbin/geom/class/raid3/geom_raid3.c ============================================================================== --- stable/10/sbin/geom/class/raid3/geom_raid3.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/raid3/geom_raid3.c Sat Mar 10 04:17:01 2018 (r330737) @@ -149,6 +149,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."); Modified: stable/10/sbin/geom/class/shsec/geom_shsec.c ============================================================================== --- stable/10/sbin/geom/class/shsec/geom_shsec.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/shsec/geom_shsec.c Sat Mar 10 04:17:01 2018 (r330737) @@ -110,6 +110,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."); Modified: stable/10/sbin/geom/class/stripe/geom_stripe.c ============================================================================== --- stable/10/sbin/geom/class/stripe/geom_stripe.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/stripe/geom_stripe.c Sat Mar 10 04:17:01 2018 (r330737) @@ -128,6 +128,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."); Modified: stable/10/sbin/geom/class/virstor/geom_virstor.c ============================================================================== --- stable/10/sbin/geom/class/virstor/geom_virstor.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/class/virstor/geom_virstor.c Sat Mar 10 04:17:01 2018 (r330737) @@ -183,6 +183,7 @@ my_g_metadata_store(const char *name, u_char *md, size goto out; } bcopy(md, sector, size); + bzero(sector + size, sectorsize - size); if (pwrite(fd, sector, sectorsize, mediasize - sectorsize) != (ssize_t)sectorsize) { error = errno; Modified: stable/10/sbin/geom/misc/subr.c ============================================================================== --- stable/10/sbin/geom/misc/subr.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sbin/geom/misc/subr.c Sat Mar 10 04:17:01 2018 (r330737) @@ -271,6 +271,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) { @@ -302,6 +309,7 @@ g_metadata_store(const char *name, const unsigned char goto out; } bcopy(md, sector, size); + bzero(sector + size, sectorsize - size); if (pwrite(fd, sector, sectorsize, mediasize - sectorsize) != sectorsize) { error = errno; Modified: stable/10/sys/geom/eli/g_eli_integrity.c ============================================================================== --- stable/10/sys/geom/eli/g_eli_integrity.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sys/geom/eli/g_eli_integrity.c Sat Mar 10 04:17:01 2018 (r330737) @@ -468,8 +468,16 @@ g_eli_auth_run(struct g_eli_worker *wr, struct bio *bp iov = (struct iovec *)p; p += sizeof(*iov); data_secsize = sc->sc_data_per_sector; - if ((i % lsec) == 0) + if ((i % lsec) == 0) { data_secsize = decr_secsize % data_secsize; + /* + * Last encrypted sector of each decrypted sector is + * only partially filled. + */ + if (bp->bio_cmd == BIO_WRITE) + memset(data + sc->sc_alen + data_secsize, 0, + encr_secsize - sc->sc_alen - data_secsize); + } if (bp->bio_cmd == BIO_READ) { /* Remember read HMAC. */ Modified: stable/10/sys/geom/virstor/g_virstor.c ============================================================================== --- stable/10/sys/geom/virstor/g_virstor.c Sat Mar 10 04:10:57 2018 (r330736) +++ stable/10/sys/geom/virstor/g_virstor.c Sat Mar 10 04:17:01 2018 (r330737) @@ -1044,6 +1044,7 @@ write_metadata(struct g_consumer *cp, struct g_virstor 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,