Bug 222077 - geli(8) writing uninitialized memory out to disk
Summary: geli(8) writing uninitialized memory out to disk
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-05 18:00 UTC by Maxim Khitrov
Modified: 2018-03-10 04:18 UTC (History)
4 users (show)

See Also:
asomers: mfc-stable11+
asomers: mfc-stable10+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Maxim Khitrov 2017-09-05 18:00:52 UTC
When geli(8) is configured to use authenticated encryption, it has to interleave data with HMACs. A 4096-byte encrypted sector requires nine 512-byte sectors from the underlying provider, with the last sector having 224 bytes unused.

I was configuring an encrypted vnode-type memory disk and decided to look at the metadata that geli writes to the end of the file with hexdump. To my surprise, I also found other plain-text data in the 224 unused bytes of every 9th sector. For example (hex columns omitted):

000ff520  |om>.    <geom id|
000ff530  |="XXXXXXXXXXXXXX|
000ff540  |XXXX">.      <cl|
000ff550  |ass ref="XXXXXXX|
000ff560  |XXXXXXXXXXX"/>. |
000ff570  |     <name>diski|
000ff580  |d/XXXXXXXXXXXXX<|
000ff590  |/name>.      <ra|
000ff5a0  |nk>3</rank>..<co|
000ff5b0  |nsumer id="XXXXX|
000ff5c0  |XXXXXXXXXXXXX">.|
000ff5d0  |.  <geom ref="XX|
000ff5e0  |XXXXXXXXXXXXXXXX|
000ff5f0  |"/>..  <provider|
000ff600  |................|

This is a portion of sysctl kern.geom.confxml (with real data replaced by Xs) that made its way into my memory disk file. The script below can be used to reproduce this problem, though the actual data will obviously vary. It may be necessary to increase the memory disk size for this behavior to become apparent.

It looks like geli is obtaining some uninitialized memory for the last output sector and not erasing the unused bytes before writing the sector out to disk. I took a brief look at the code, but couldn't figure out where the memory is coming from, so I'm not sure of the severity of this bug and whether it may expose some sensitive data.

Either way, the uninitialized bytes should be either set to zero or replaced with random data to make them indistinguishable from the rest (though there is probably no real security benefit from doing so because of the geli metadata at the end of the device).

Script to create an authenticated memory disk (use `hd gelitest.md` to inspect contents):

#!/bin/sh
dd if=/dev/zero of=gelitest.md bs=1M count=1 status=none
md=$(mdconfig -f gelitest.md) || exit
echo fakekey | geli init -a HMAC/SHA256 -B none -K - -P -s 4096 $md || exit
echo fakekey | geli attach -k - -p $md || exit
dd if=/dev/zero of=/dev/$md.eli bs=1M status=none
geli detach /dev/$md.eli
mdconfig -du $md
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2017-09-05 19:12:50 UTC
This is almost certainly a severe security issue.  Thanks for reporting it!
Comment 2 Conrad Meyer freebsd_committer freebsd_triage 2017-09-05 19:47:22 UTC
It seems like the math in g_eli_auth_run (g_eli_integrity.c) is kind of dubious, even ignoring the data leak.  Does it even work?  I think it is making some invalid assumptions about C division rounding.  It looks like the logic was copied from g_eli_crypto_run (g_eli_privacy.c).

I suspect this change wouldn't hurt, but I don't think it fixes the problem:

--- a/sys/geom/eli/g_eli_integrity.c
+++ b/sys/geom/eli/g_eli_integrity.c
@@ -445,7 +445,7 @@ g_eli_auth_run(struct g_eli_worker *wr, struct bio *bp)
                size += sizeof(*crda) * nsec;
                size += G_ELI_AUTH_SECKEYLEN * nsec;
                size += sizeof(uintptr_t);      /* Space for alignment. */
-               data = malloc(size, M_ELI, M_WAITOK);
+               data = malloc(size, M_ELI, M_WAITOK | M_ZERO);
                bp->bio_driver2 = data;
                p = data + encr_secsize * nsec;
        }

I think "nsec" is calculated wrong (and differently!) in both g_eli_auth_write_done and g_eli_auth_run.

Probably avoid using geli in integrity mode if you care about privacy, for now.
Comment 3 Maxim Khitrov 2017-09-07 15:46:12 UTC
I think the same issue also affects geli metadata with 4k sector size. I was able to reproduce this using the script below, but it took many iterations. Once it happened, the same data was returned every time, so I think it's just a matter of getting the right memory page allocated. The result is that the last sector contains 512 bytes of metadata followed by 3584 bytes of uninitialized memory.

#!/bin/sh
dd if=/dev/zero of=gelitest.md bs=8K count=1 status=none
md=$(mdconfig -f gelitest.md -S 4096) || exit
echo 'fakekey' | geli init -B none -K - -P $md || exit
mdconfig -du $md
hd gelitest.md
Comment 4 Conrad Meyer freebsd_committer freebsd_triage 2017-09-07 16:06:32 UTC
(In reply to Maxim Khitrov from comment #3)
This issue is a userspace leak.  It comes from g_metadata_store() in the geom userspace code:

        fd = g_open(name, 1);
...
        sectorsize = g_sectorsize(fd);   // E.g., 4096
...
        assert(sectorsize >= (ssize_t)size);    // size == metadata size, e.g., 512
        sector = malloc(sectorsize);     // malloc doesn't zero contents
...
        bcopy(md, sector, size);         // only first size bytes are initialized
        if (pwrite(fd, sector, sectorsize, mediasize - sectorsize) !=
            sectorsize) {
                                         // sectorsize bytes are written
Comment 5 Conrad Meyer freebsd_committer freebsd_triage 2017-09-07 16:29:23 UTC
https://reviews.freebsd.org/D12269 is for the comment 3 issue.  Does not address the issue in the original description.
Comment 6 Conrad Meyer freebsd_committer freebsd_triage 2017-09-08 05:21:36 UTC
https://reviews.freebsd.org/D12272 is for the main issue.
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-09-08 15:09:12 UTC
A commit references this bug:

Author: cem
Date: Fri Sep  8 15:08:18 UTC 2017
New revision: 323314
URL: https://svnweb.freebsd.org/changeset/base/323314

Log:
  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 <max AT mxcrypt.com>
  Reviewed by:	des
  Security:	yes
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D12269

Changes:
  head/sbin/geom/class/virstor/geom_virstor.c
  head/sbin/geom/misc/subr.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-09-09 01:41:35 UTC
A commit references this bug:

Author: cem
Date: Sat Sep  9 01:41:01 UTC 2017
New revision: 323338
URL: https://svnweb.freebsd.org/changeset/base/323338

Log:
  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 <max AT mxcrypt.com>
  Reviewed by:	emaste
  Security:	yes
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D12272

Changes:
  head/sys/geom/eli/g_eli_integrity.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2018-02-04 14:50:24 UTC
A commit references this bug:

Author: asomers
Date: Sun Feb  4 14:49:56 UTC 2018
New revision: 328849
URL: https://svnweb.freebsd.org/changeset/base/328849

Log:
  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 <max@mxcrypt.com>
  Reviewed by:	cem
  MFC after:	3 weeks
  X-MFC-With:	323314
  X-MFC-With:	323338
  Differential Revision:	https://reviews.freebsd.org/D14164

Changes:
  head/sbin/geom/class/cache/geom_cache.c
  head/sbin/geom/class/concat/geom_concat.c
  head/sbin/geom/class/journal/geom_journal.c
  head/sbin/geom/class/label/geom_label.c
  head/sbin/geom/class/mirror/geom_mirror.c
  head/sbin/geom/class/raid3/geom_raid3.c
  head/sbin/geom/class/shsec/geom_shsec.c
  head/sbin/geom/class/stripe/geom_stripe.c
  head/sbin/geom/misc/subr.c
  head/sys/geom/virstor/g_virstor.c
Comment 10 Alan Somers freebsd_committer freebsd_triage 2018-02-04 14:50:48 UTC
Reopening for an MFC.
Comment 11 Conrad Meyer freebsd_committer freebsd_triage 2018-02-22 21:48:35 UTC
Bouncing assignee for reopen.
Comment 12 commit-hook freebsd_committer freebsd_triage 2018-03-10 02:16:25 UTC
A commit references this bug:

Author: asomers
Date: Sat Mar 10 02:15:47 UTC 2018
New revision: 330726
URL: https://svnweb.freebsd.org/changeset/base/330726

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 <max AT mxcrypt.com>
  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 <max AT mxcrypt.com>
  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 <max@mxcrypt.com>
  Reviewed by:	cem
  X-MFC-With:	323314
  X-MFC-With:	323338
  Differential Revision:	https://reviews.freebsd.org/D14164

Changes:
_U  stable/11/
  stable/11/sbin/geom/class/cache/geom_cache.c
  stable/11/sbin/geom/class/concat/geom_concat.c
  stable/11/sbin/geom/class/journal/geom_journal.c
  stable/11/sbin/geom/class/label/geom_label.c
  stable/11/sbin/geom/class/mirror/geom_mirror.c
  stable/11/sbin/geom/class/raid3/geom_raid3.c
  stable/11/sbin/geom/class/shsec/geom_shsec.c
  stable/11/sbin/geom/class/stripe/geom_stripe.c
  stable/11/sbin/geom/class/virstor/geom_virstor.c
  stable/11/sbin/geom/misc/subr.c
  stable/11/sys/geom/eli/g_eli_integrity.c
  stable/11/sys/geom/virstor/g_virstor.c
Comment 13 commit-hook freebsd_committer freebsd_triage 2018-03-10 02:16:30 UTC
A commit references this bug:

Author: asomers
Date: Sat Mar 10 02:15:47 UTC 2018
New revision: 330726
URL: https://svnweb.freebsd.org/changeset/base/330726

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 <max AT mxcrypt.com>
  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 <max AT mxcrypt.com>
  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 <max@mxcrypt.com>
  Reviewed by:	cem
  X-MFC-With:	323314
  X-MFC-With:	323338
  Differential Revision:	https://reviews.freebsd.org/D14164

Changes:
_U  stable/11/
  stable/11/sbin/geom/class/cache/geom_cache.c
  stable/11/sbin/geom/class/concat/geom_concat.c
  stable/11/sbin/geom/class/journal/geom_journal.c
  stable/11/sbin/geom/class/label/geom_label.c
  stable/11/sbin/geom/class/mirror/geom_mirror.c
  stable/11/sbin/geom/class/raid3/geom_raid3.c
  stable/11/sbin/geom/class/shsec/geom_shsec.c
  stable/11/sbin/geom/class/stripe/geom_stripe.c
  stable/11/sbin/geom/class/virstor/geom_virstor.c
  stable/11/sbin/geom/misc/subr.c
  stable/11/sys/geom/eli/g_eli_integrity.c
  stable/11/sys/geom/virstor/g_virstor.c
Comment 14 commit-hook freebsd_committer freebsd_triage 2018-03-10 02:16:32 UTC
A commit references this bug:

Author: asomers
Date: Sat Mar 10 02:15:47 UTC 2018
New revision: 330726
URL: https://svnweb.freebsd.org/changeset/base/330726

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 <max AT mxcrypt.com>
  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 <max AT mxcrypt.com>
  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 <max@mxcrypt.com>
  Reviewed by:	cem
  X-MFC-With:	323314
  X-MFC-With:	323338
  Differential Revision:	https://reviews.freebsd.org/D14164

Changes:
_U  stable/11/
  stable/11/sbin/geom/class/cache/geom_cache.c
  stable/11/sbin/geom/class/concat/geom_concat.c
  stable/11/sbin/geom/class/journal/geom_journal.c
  stable/11/sbin/geom/class/label/geom_label.c
  stable/11/sbin/geom/class/mirror/geom_mirror.c
  stable/11/sbin/geom/class/raid3/geom_raid3.c
  stable/11/sbin/geom/class/shsec/geom_shsec.c
  stable/11/sbin/geom/class/stripe/geom_stripe.c
  stable/11/sbin/geom/class/virstor/geom_virstor.c
  stable/11/sbin/geom/misc/subr.c
  stable/11/sys/geom/eli/g_eli_integrity.c
  stable/11/sys/geom/virstor/g_virstor.c
Comment 15 commit-hook freebsd_committer freebsd_triage 2018-03-10 04:17:30 UTC
A commit references this bug:

Author: asomers
Date: Sat Mar 10 04:17:02 UTC 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 <max AT mxcrypt.com>
  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 <max AT mxcrypt.com>
  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 <max@mxcrypt.com>
  Reviewed by:	cem
  X-MFC-With:	323314
  X-MFC-With:	323338
  Differential Revision:	https://reviews.freebsd.org/D14164

Changes:
_U  stable/10/
  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
Comment 16 commit-hook freebsd_committer freebsd_triage 2018-03-10 04:17:35 UTC
A commit references this bug:

Author: asomers
Date: Sat Mar 10 04:17:02 UTC 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 <max AT mxcrypt.com>
  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 <max AT mxcrypt.com>
  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 <max@mxcrypt.com>
  Reviewed by:	cem
  X-MFC-With:	323314
  X-MFC-With:	323338
  Differential Revision:	https://reviews.freebsd.org/D14164

Changes:
_U  stable/10/
  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
Comment 17 commit-hook freebsd_committer freebsd_triage 2018-03-10 04:17:37 UTC
A commit references this bug:

Author: asomers
Date: Sat Mar 10 04:17:02 UTC 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 <max AT mxcrypt.com>
  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 <max AT mxcrypt.com>
  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 <max@mxcrypt.com>
  Reviewed by:	cem
  X-MFC-With:	323314
  X-MFC-With:	323338
  Differential Revision:	https://reviews.freebsd.org/D14164

Changes:
_U  stable/10/
  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