Author: cem
Date: Tue Jun  4 00:01:37 2019
New Revision: 348595
URL: https://svnweb.freebsd.org/changeset/base/348595

Log:
  virtio_random(4): Fix random(4) integration
  
  random(4) masks unregistered entropy sources.  Prior to this revision,
  virtio_random(4) did not correctly register a random_source and did not
  function as a source of entropy.
  
  Random source registration for loadable pure sources requires registering a
  poll callback, which is invoked periodically by random(4)'s harvestq
  kthread.  The periodic poll makes virtio_random(4)'s periodic entropy
  collection redundant, so this revision removes the callout.
  
  The current random source API is somewhat limiting, so simply fail to attach
  any virtio_random devices if one is already registered as a source.  This
  scenario is expected to be uncommon.
  
  While here, handle the possibility of short reads from the hypervisor random
  device gracefully / correctly.  It is not clear why a hypervisor would
  return a short read or if it is allowed by spec, but we may as well handle
  it.
  
  Reviewed by:  bryanv (earlier version), markm
  Security:     yes (note: many other "pure" random sources remain broken)
  Sponsored by: Dell EMC Isilon
  Differential Revision:        https://reviews.freebsd.org/D20419

Modified:
  head/sys/dev/virtio/random/virtio_random.c

Modified: head/sys/dev/virtio/random/virtio_random.c
==============================================================================
--- head/sys/dev/virtio/random/virtio_random.c  Mon Jun  3 23:57:29 2019        
(r348594)
+++ head/sys/dev/virtio/random/virtio_random.c  Tue Jun  4 00:01:37 2019        
(r348595)
@@ -33,21 +33,24 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/param.h>
 #include <sys/kernel.h>
+#include <sys/malloc.h>
 #include <sys/module.h>
 #include <sys/sglist.h>
 #include <sys/callout.h>
 #include <sys/random.h>
+#include <sys/stdatomic.h>
 
 #include <machine/bus.h>
 #include <machine/resource.h>
 #include <sys/bus.h>
 
+#include <dev/random/randomdev.h>
+#include <dev/random/random_harvestq.h>
 #include <dev/virtio/virtio.h>
 #include <dev/virtio/virtqueue.h>
 
 struct vtrnd_softc {
        uint64_t                 vtrnd_features;
-       struct callout           vtrnd_callout;
        struct virtqueue        *vtrnd_vq;
 };
 
@@ -59,8 +62,8 @@ static int    vtrnd_detach(device_t);
 
 static void    vtrnd_negotiate_features(device_t);
 static int     vtrnd_alloc_virtqueue(device_t);
-static void    vtrnd_harvest(struct vtrnd_softc *);
-static void    vtrnd_timer(void *);
+static int     vtrnd_harvest(struct vtrnd_softc *, void *, size_t *);
+static unsigned        vtrnd_read(void *, unsigned);
 
 #define VTRND_FEATURES 0
 
@@ -68,6 +71,15 @@ static struct virtio_feature_desc vtrnd_feature_desc[]
        { 0, NULL }
 };
 
+static struct random_source random_vtrnd = {
+       .rs_ident = "VirtIO Entropy Adapter",
+       .rs_source = RANDOM_PURE_VIRTIO,
+       .rs_read = vtrnd_read,
+};
+
+/* Kludge for API limitations of random(4). */
+static _Atomic(struct vtrnd_softc *) g_vtrnd_softc;
+
 static device_method_t vtrnd_methods[] = {
        /* Device methods. */
        DEVMETHOD(device_probe,         vtrnd_probe),
@@ -125,13 +137,11 @@ vtrnd_probe(device_t dev)
 static int
 vtrnd_attach(device_t dev)
 {
-       struct vtrnd_softc *sc;
+       struct vtrnd_softc *sc, *exp;
        int error;
 
        sc = device_get_softc(dev);
 
-       callout_init(&sc->vtrnd_callout, 1);
-
        virtio_set_feature_desc(dev, vtrnd_feature_desc);
        vtrnd_negotiate_features(dev);
 
@@ -141,7 +151,13 @@ vtrnd_attach(device_t dev)
                goto fail;
        }
 
-       callout_reset(&sc->vtrnd_callout, 5 * hz, vtrnd_timer, sc);
+       exp = NULL;
+       if (!atomic_compare_exchange_strong_explicit(&g_vtrnd_softc, &exp, sc,
+           memory_order_release, memory_order_acquire)) {
+               error = EEXIST;
+               goto fail;
+       }
+       random_source_register(&random_vtrnd);
 
 fail:
        if (error)
@@ -156,9 +172,20 @@ vtrnd_detach(device_t dev)
        struct vtrnd_softc *sc;
 
        sc = device_get_softc(dev);
+       KASSERT(
+           atomic_load_explicit(&g_vtrnd_softc, memory_order_acquire) == sc,
+           ("only one global instance at a time"));
 
-       callout_drain(&sc->vtrnd_callout);
+       random_source_deregister(&random_vtrnd);
+       atomic_store_explicit(&g_vtrnd_softc, NULL, memory_order_release);
 
+       /*
+        * Unfortunately, deregister does not guarantee our source callback
+        * will not be invoked after it returns.  Use a kludge to prevent some,
+        * but not all, possible races.
+        */
+       tsleep_sbt(&g_vtrnd_softc, 0, "vtrnddet", mstosbt(50), 0, C_HARDCLOCK);
+
        return (0);
 }
 
@@ -185,44 +212,64 @@ vtrnd_alloc_virtqueue(device_t dev)
        return (virtio_alloc_virtqueues(dev, 0, 1, &vq_info));
 }
 
-static void
-vtrnd_harvest(struct vtrnd_softc *sc)
+static int
+vtrnd_harvest(struct vtrnd_softc *sc, void *buf, size_t *sz)
 {
        struct sglist_seg segs[1];
        struct sglist sg;
        struct virtqueue *vq;
-       uint32_t value;
+       uint32_t value[HARVESTSIZE] __aligned(sizeof(uint32_t) * HARVESTSIZE);
+       uint32_t rdlen;
        int error;
 
-       vq = sc->vtrnd_vq;
+       _Static_assert(sizeof(value) < PAGE_SIZE, "sglist assumption");
 
        sglist_init(&sg, 1, segs);
-       error = sglist_append(&sg, &value, sizeof(value));
-       KASSERT(error == 0 && sg.sg_nseg == 1,
-           ("%s: error %d adding buffer to sglist", __func__, error));
+       error = sglist_append(&sg, value, *sz);
+       if (error != 0)
+               panic("%s: sglist_append error=%d", __func__, error);
 
-       if (!virtqueue_empty(vq))
-               return;
-       if (virtqueue_enqueue(vq, &value, &sg, 0, 1) != 0)
-               return;
+       vq = sc->vtrnd_vq;
+       KASSERT(virtqueue_empty(vq), ("%s: non-empty queue", __func__));
 
+       error = virtqueue_enqueue(vq, buf, &sg, 0, 1);
+       if (error != 0)
+               return (error);
+
        /*
         * Poll for the response, but the command is likely already
         * done when we return from the notify.
         */
        virtqueue_notify(vq);
-       virtqueue_poll(vq, NULL);
+       virtqueue_poll(vq, &rdlen);
 
-       random_harvest_queue(&value, sizeof(value), RANDOM_PURE_VIRTIO);
+       if (rdlen > *sz)
+               panic("%s: random device wrote %zu bytes beyond end of provided"
+                   " buffer %p:%zu", __func__, (size_t)rdlen - *sz,
+                   (void *)value, *sz);
+       else if (rdlen == 0)
+               return (EAGAIN);
+       *sz = MIN(rdlen, *sz);
+       memcpy(buf, value, *sz);
+       explicit_bzero(value, *sz);
+       return (0);
 }
 
-static void
-vtrnd_timer(void *xsc)
+static unsigned
+vtrnd_read(void *buf, unsigned usz)
 {
        struct vtrnd_softc *sc;
+       size_t sz;
+       int error;
 
-       sc = xsc;
+       sc = g_vtrnd_softc;
+       if (sc == NULL)
+               return (0);
 
-       vtrnd_harvest(sc);
-       callout_schedule(&sc->vtrnd_callout, 5 * hz);
+       sz = usz;
+       error = vtrnd_harvest(sc, buf, &sz);
+       if (error != 0)
+               return (0);
+
+       return (sz);
 }
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to