svn commit: r355137 - head/sys/vm

[Ryan Libby]

On Tue, Dec 3, 2019 at 12:43 PM Gleb Smirnoff <glebius at freebsd.org> wrote:
>
>   Ryan,
>
> On Wed, Nov 27, 2019 at 07:49:56PM +0000, Ryan Libby wrote:
> R> Author: rlibby
> R> Date: Wed Nov 27 19:49:55 2019
> R> New Revision: 355137
> R> URL: https://svnweb.freebsd.org/changeset/base/355137
> R>
> R> Log:
> R>   uma: trash memory when ctor/dtor supplied too
> R>
> R>   On INVARIANTS kernels, UMA has a use-after-free detection mechanism.
> R>   This mechanism previously required that all of the ctor/dtor/uminit/fini
> R>   arguments to uma_zcreate() be NULL in order to function.  Now, it only
> R>   requires that uminit and fini be NULL; now, the trash ctor and dtor will
> R>   be called in addition to any supplied ctor or dtor.
> R>
> R>   Also do a little refactoring for readability of the resulting logic.
> R>
> R>   This enables use-after-free detection for more zones, and will allow for
> R>   simplification of some callers that worked around the previous
> R>   restriction (see kern_mbuf.c).
> R>
> R>   Reviewed by:       jeff, markj
> R>   Sponsored by:      Dell EMC Isilon
> R>   Differential Revision:     https://reviews.freebsd.org/D20722
>
> If I understand the change correct, now items from UMA_ZONE_NOFREE zones
> will be trashed, too. That would undermine purpose of UMA_ZONE_NOFREE.
> Of course the flag is a hack, but some systems rely on it working.
>
> --
> Gleb Smirnoff

The intent is not to change anything for NOFREE zones (i.e. still don't
trash them).  I didn't put all the detail in the commit log, but I did
reword the block comment in uma_zcreate:

> @@ -2302,14 +2307,17 @@ uma_zcreate(const char *name, size_t size, uma_ctor ct
>         args.fini = fini;
>  #ifdef  INVARIANTS
>         /*
> -        * If a zone is being created with an empty constructor and
> -        * destructor, pass UMA constructor/destructor which checks for
> -        * memory use after free.
> +        * Inject procedures which check for memory use after free if we are
> +        * allowed to scramble the memory while it is not allocated.  This
> +        * requires that: UMA is actually able to access the memory, no init
> +        * or fini procedures, no dependency on the initial value of the
> +        * memory, and no (legitimate) use of the memory after free.  Note,
> +        * the ctor and dtor do not need to be empty.
> +        *
> +        * XXX UMA_ZONE_OFFPAGE.
>          */
>         if ((!(flags & (UMA_ZONE_ZINIT | UMA_ZONE_NOFREE))) &&
> -           ctor == NULL && dtor == NULL && uminit == NULL && fini == NULL) {
> -               args.ctor = trash_ctor;
> -               args.dtor = trash_dtor;
> +           uminit == NULL && fini == NULL) {
>                 args.uminit = trash_init;
>                 args.fini = trash_fini;
>         }

Ryan