Author: rlibby
Date: Wed Nov 27 19:49:55 2019
New Revision: 355137
URL: https://svnweb.freebsd.org/changeset/base/355137

Log:
  uma: trash memory when ctor/dtor supplied too
  
  On INVARIANTS kernels, UMA has a use-after-free detection mechanism.
  This mechanism previously required that all of the ctor/dtor/uminit/fini
  arguments to uma_zcreate() be NULL in order to function.  Now, it only
  requires that uminit and fini be NULL; now, the trash ctor and dtor will
  be called in addition to any supplied ctor or dtor.
  
  Also do a little refactoring for readability of the resulting logic.
  
  This enables use-after-free detection for more zones, and will allow for
  simplification of some callers that worked around the previous
  restriction (see kern_mbuf.c).
  
  Reviewed by:  jeff, markj
  Sponsored by: Dell EMC Isilon
  Differential Revision:        https://reviews.freebsd.org/D20722

Modified:
  head/sys/vm/uma_core.c
  head/sys/vm/uma_int.h

Modified: head/sys/vm/uma_core.c
==============================================================================
--- head/sys/vm/uma_core.c      Wed Nov 27 19:34:33 2019        (r355136)
+++ head/sys/vm/uma_core.c      Wed Nov 27 19:49:55 2019        (r355137)
@@ -1869,6 +1869,11 @@ zone_ctor(void *mem, int size, void *udata, int flags)
        for (i = 0; i < vm_ndomains; i++)
                TAILQ_INIT(&zone->uz_domain[i].uzd_buckets);
 
+#ifdef INVARIANTS
+       if (arg->uminit == trash_init && arg->fini == trash_fini)
+               zone->uz_flags |= UMA_ZFLAG_TRASH;
+#endif
+
        /*
         * This is a pure cache zone, no kegs.
         */
@@ -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;
        }
@@ -2462,15 +2470,14 @@ static void *
 item_ctor(uma_zone_t zone, void *udata, int flags, void *item)
 {
 #ifdef INVARIANTS
-       int skipdbg;
+       bool skipdbg;
 
        skipdbg = uma_dbg_zskip(zone, item);
-       if (zone->uz_ctor != NULL &&
-           (!skipdbg || zone->uz_ctor != trash_ctor ||
-           zone->uz_dtor != trash_dtor) &&
-#else
-       if (__predict_false(zone->uz_ctor != NULL) &&
+       if (!skipdbg && (zone->uz_flags & UMA_ZFLAG_TRASH) != 0 &&
+           zone->uz_ctor != trash_ctor)
+               trash_ctor(item, zone->uz_size, udata, flags);
 #endif
+       if (__predict_false(zone->uz_ctor != NULL) &&
            zone->uz_ctor(item, zone->uz_size, udata, flags) != 0) {
                counter_u64_add(zone->uz_fails, 1);
                zone_free_item(zone, item, udata, SKIP_DTOR | SKIP_CNT);
@@ -2486,6 +2493,31 @@ item_ctor(uma_zone_t zone, void *udata, int flags, voi
        return (item);
 }
 
+static inline void
+item_dtor(uma_zone_t zone, void *item, void *udata, enum zfreeskip skip)
+{
+#ifdef INVARIANTS
+       bool skipdbg;
+
+       skipdbg = uma_dbg_zskip(zone, item);
+       if (skip == SKIP_NONE && !skipdbg) {
+               if ((zone->uz_flags & UMA_ZONE_MALLOC) != 0)
+                       uma_dbg_free(zone, udata, item);
+               else
+                       uma_dbg_free(zone, NULL, item);
+       }
+#endif
+       if (skip < SKIP_DTOR) {
+               if (zone->uz_dtor != NULL)
+                       zone->uz_dtor(item, zone->uz_size, udata);
+#ifdef INVARIANTS
+               if (!skipdbg && (zone->uz_flags & UMA_ZFLAG_TRASH) != 0 &&
+                   zone->uz_dtor != trash_dtor)
+                       trash_dtor(item, zone->uz_size, udata);
+#endif
+       }
+}
+
 /* See uma.h */
 void *
 uma_zalloc_arg(uma_zone_t zone, void *udata, int flags)
@@ -2523,6 +2555,7 @@ uma_zalloc_arg(uma_zone_t zone, void *udata, int flags
                        if (zone->uz_ctor != NULL &&
                            zone->uz_ctor(item, zone->uz_size, udata,
                            flags) != 0) {
+                               counter_u64_add(zone->uz_fails, 1);
                                zone->uz_fini(item, zone->uz_size);
                                return (NULL);
                        }
@@ -3131,9 +3164,6 @@ uma_zfree_arg(uma_zone_t zone, void *item, void *udata
        int itemdomain;
 #endif
        bool lockfail;
-#ifdef INVARIANTS
-       bool skipdbg;
-#endif
 
        /* Enable entropy collection for RANDOM_ENABLE_UMA kernel option */
        random_harvest_fast_uma(&zone, sizeof(zone), RANDOM_UMA);
@@ -3157,20 +3187,7 @@ uma_zfree_arg(uma_zone_t zone, void *item, void *udata
                return;
        }
 #endif
-#ifdef INVARIANTS
-       skipdbg = uma_dbg_zskip(zone, item);
-       if (skipdbg == false) {
-               if (zone->uz_flags & UMA_ZONE_MALLOC)
-                       uma_dbg_free(zone, udata, item);
-               else
-                       uma_dbg_free(zone, NULL, item);
-       }
-       if (zone->uz_dtor != NULL && (!skipdbg ||
-           zone->uz_dtor != trash_dtor || zone->uz_ctor != trash_ctor))
-#else
-       if (zone->uz_dtor != NULL)
-#endif
-               zone->uz_dtor(item, zone->uz_size, udata);
+       item_dtor(zone, item, udata, SKIP_NONE);
 
        /*
         * The race here is acceptable.  If we miss it we'll just have to wait
@@ -3459,24 +3476,8 @@ zone_release(uma_zone_t zone, void **bucket, int cnt)
 static void
 zone_free_item(uma_zone_t zone, void *item, void *udata, enum zfreeskip skip)
 {
-#ifdef INVARIANTS
-       bool skipdbg;
 
-       skipdbg = uma_dbg_zskip(zone, item);
-       if (skip == SKIP_NONE && !skipdbg) {
-               if (zone->uz_flags & UMA_ZONE_MALLOC)
-                       uma_dbg_free(zone, udata, item);
-               else
-                       uma_dbg_free(zone, NULL, item);
-       }
-
-       if (skip < SKIP_DTOR && zone->uz_dtor != NULL &&
-           (!skipdbg || zone->uz_dtor != trash_dtor ||
-           zone->uz_ctor != trash_ctor))
-#else
-       if (skip < SKIP_DTOR && zone->uz_dtor != NULL)
-#endif
-               zone->uz_dtor(item, zone->uz_size, udata);
+       item_dtor(zone, item, udata, skip);
 
        if (skip < SKIP_FINI && zone->uz_fini)
                zone->uz_fini(item, zone->uz_size);

Modified: head/sys/vm/uma_int.h
==============================================================================
--- head/sys/vm/uma_int.h       Wed Nov 27 19:34:33 2019        (r355136)
+++ head/sys/vm/uma_int.h       Wed Nov 27 19:49:55 2019        (r355137)
@@ -389,6 +389,7 @@ struct uma_zone {
 #define        UMA_ZFLAG_RECLAIMING    0x08000000      /* Running 
zone_reclaim(). */
 #define        UMA_ZFLAG_BUCKET        0x10000000      /* Bucket zone. */
 #define UMA_ZFLAG_INTERNAL     0x20000000      /* No offpage no PCPU. */
+#define UMA_ZFLAG_TRASH                0x40000000      /* Add trash ctor/dtor. 
*/
 #define UMA_ZFLAG_CACHEONLY    0x80000000      /* Don't ask VM for buckets. */
 
 #define        UMA_ZFLAG_INHERIT                                               
\
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to