svn commit: r323314 - in head/sbin/geom: class/virstor misc

[Alan Somers]

On Fri, Sep 8, 2017 at 9:08 AM, Conrad Meyer <cem at freebsd.org> wrote:

> Author: cem
> Date: Fri Sep  8 15:08:17 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
>
> Modified:
>   head/sbin/geom/class/virstor/geom_virstor.c
>   head/sbin/geom/misc/subr.c
>

The problem is real, but the change doesn't completely fix the problem.
The problem is that classes don't use g_metadata_store the way you
assumed.  Instead of the size argument indicating the useful portion of the
label, most if not all classes use it to indicate the size of their buffer,
and they pass in a buffer that hasn't been bzero()ed.  Fixing this bug will
require changes to each class.  For example, geom_label.c creates a 512B
buffer, initializes the first 44 bytes, then calls g_metadata_store with
size=512.  I'll work on a fix tomorrow.

-Alan