Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/pci - rnd_add_*() call must be serialized. Unseriali...



details:   https://anonhg.NetBSD.org/src/rev/2d697fdafead
branches:  trunk
changeset: 458093:2d697fdafead
user:      msaitoh <msaitoh%NetBSD.org@localhost>
date:      Tue Jul 30 04:42:29 2019 +0000

description:
- rnd_add_*() call must be serialized. Unserialized accesses broke the
  sc->rnd_source structure and caused off by one in kern_rndq.c's
  rnd_add_data_ts(). This problem was found by kUBSan. Make krndsource_t's
  data per queue-pair and call rnd_add_uint32() in wm_txrxintr_msix(). One of
  the reason why rnd_add_uint32() calls were moved from wm_{tx,rx}eof() is that
  rnd(9) manual page says "To allow the system to gather the timing information
  accurately, this call should be placed within the actual hardware interrupt
  service routine."
- Pass a better value to rnd_add_uint32() in wm_intr_legacy().

XXX pullup-8.

diffstat:

 sys/dev/pci/if_wm.c |  50 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 34 insertions(+), 16 deletions(-)

diffs (173 lines):

diff -r 2a3dc20943f4 -r 2d697fdafead sys/dev/pci/if_wm.c
--- a/sys/dev/pci/if_wm.c       Mon Jul 29 17:53:20 2019 +0000
+++ b/sys/dev/pci/if_wm.c       Tue Jul 30 04:42:29 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_wm.c,v 1.644 2019/07/26 09:26:56 msaitoh Exp $      */
+/*     $NetBSD: if_wm.c,v 1.645 2019/07/30 04:42:29 msaitoh Exp $      */
 
 /*
  * Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -82,7 +82,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.644 2019/07/26 09:26:56 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.645 2019/07/30 04:42:29 msaitoh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -234,7 +234,7 @@
  * packet.  We allocate 256 receive descriptors, each with a 2k
  * buffer (MCLBYTES), which gives us room for 50 jumbo packets.
  */
-#define        WM_NRXDESC              256
+#define        WM_NRXDESC              256U
 #define        WM_NRXDESC_MASK         (WM_NRXDESC - 1)
 #define        WM_NEXTRX(x)            (((x) + 1) & WM_NRXDESC_MASK)
 #define        WM_PREVRX(x)            (((x) - 1) & WM_NRXDESC_MASK)
@@ -458,6 +458,7 @@
        struct wm_rxqueue wmq_rxq;
 
        void *wmq_si;
+       krndsource_t rnd_source;        /* random source */
 };
 
 struct wm_phyop {
@@ -587,8 +588,6 @@
 
        int sc_mchash_type;             /* multicast filter offset */
 
-       krndsource_t rnd_source;        /* random source */
-
        struct if_percpuq *sc_ipq;      /* softint-based input queues */
 
        kmutex_t *sc_core_lock;         /* lock for softc operations */
@@ -2956,8 +2955,6 @@
        ether_ifattach(ifp, enaddr);
        ether_set_ifflags_cb(&sc->sc_ethercom, wm_ifflags_cb);
        if_register(ifp);
-       rnd_attach_source(&sc->rnd_source, xname, RND_TYPE_NET,
-           RND_FLAG_DEFAULT);
 
 #ifdef WM_EVENT_COUNTERS
        /* Attach event counters. */
@@ -3012,8 +3009,6 @@
        evcnt_detach(&sc->sc_ev_rx_macctl);
 #endif /* WM_EVENT_COUNTERS */
 
-       rnd_detach_source(&sc->rnd_source);
-
        /* Tell the firmware about the release */
        WM_CORE_LOCK(sc);
        wm_release_manageability(sc);
@@ -6831,6 +6826,15 @@
        if (error)
                goto fail_2;
 
+       for (i = 0; i < sc->sc_nqueues; i++) {
+               char rndname[16];
+
+               snprintf(rndname, sizeof(rndname), "%sTXRX%d",
+                   device_xname(sc->sc_dev), i);
+               rnd_attach_source(&sc->sc_queue[i].rnd_source, rndname,
+                   RND_TYPE_NET, RND_FLAG_DEFAULT);
+       }
+
        return 0;
 
  fail_2:
@@ -6866,6 +6870,9 @@
 {
        int i;
 
+       for (i = 0; i < sc->sc_nqueues; i++)
+               rnd_detach_source(&sc->sc_queue[i].rnd_source);
+
        for (i = 0; i < sc->sc_nqueues; i++) {
                struct wm_rxqueue *rxq = &sc->sc_queue[i].wmq_rxq;
 
@@ -8532,9 +8539,6 @@
        DPRINTF(WM_DEBUG_TX,
            ("%s: TX: txsdirty -> %d\n", device_xname(sc->sc_dev), i));
 
-       if (count != 0)
-               rnd_add_uint32(&sc->rnd_source, count);
-
        /*
         * If there are no more pending transmissions, cancel the watchdog
         * timer.
@@ -8948,9 +8952,6 @@
                        break;
        }
 
-       if (count != 0)
-               rnd_add_uint32(&sc->rnd_source, count);
-
        DPRINTF(WM_DEBUG_RX,
            ("%s: RX: rxptr -> %d\n", device_xname(sc->sc_dev), i));
 
@@ -9416,6 +9417,8 @@
                 * So, we can call wm_rxeof() in interrupt context.
                 */
                wm_rxeof(rxq, UINT_MAX);
+               /* Fill lower bits with RX index. See below for the upper. */
+               rndval |= rxq->rxq_ptr & WM_NRXDESC_MASK;
 
                mutex_exit(rxq->rxq_lock);
                mutex_enter(txq->txq_lock);
@@ -9434,6 +9437,8 @@
                }
 #endif
                wm_txeof(txq, UINT_MAX);
+               /* Fill upper bits with TX index. See above for the lower. */
+               rndval = txq->txq_next * WM_NRXDESC;
 
                mutex_exit(txq->txq_lock);
                WM_CORE_LOCK(sc);
@@ -9458,7 +9463,7 @@
                }
        }
 
-       rnd_add_uint32(&sc->rnd_source, rndval);
+       rnd_add_uint32(&sc->sc_queue[0].rnd_source, rndval);
 
        if (handled) {
                /* Try to get more packets going. */
@@ -9515,6 +9520,7 @@
        struct wm_softc *sc = txq->txq_sc;
        u_int txlimit = sc->sc_tx_intr_process_limit;
        u_int rxlimit = sc->sc_rx_intr_process_limit;
+       uint32_t rndval = 0;
        bool txmore;
        bool rxmore;
 
@@ -9534,6 +9540,8 @@
 
        WM_Q_EVCNT_INCR(txq, txdw);
        txmore = wm_txeof(txq, txlimit);
+       /* Fill upper bits with TX index. See below for the lower. */
+       rndval = txq->txq_next * WM_NRXDESC;
        /* wm_deferred start() is done in wm_handle_queue(). */
        mutex_exit(txq->txq_lock);
 
@@ -9548,10 +9556,20 @@
 
        WM_Q_EVCNT_INCR(rxq, intr);
        rxmore = wm_rxeof(rxq, rxlimit);
+
+       /* Fill lower bits with RX index. See above for the upper. */
+       rndval |= rxq->rxq_ptr & WM_NRXDESC_MASK;
        mutex_exit(rxq->rxq_lock);
 
        wm_itrs_writereg(sc, wmq);
 
+       /*
+        * This function is called in the hardware interrupt context and
+        * per-CPU, so it's not required to take a lock.
+        */
+       if (rndval != 0)
+               rnd_add_uint32(&sc->sc_queue[wmq->wmq_id].rnd_source, rndval);
+
        if (txmore || rxmore)
                softint_schedule(wmq->wmq_si);
        else



Home | Main Index | Thread Index | Old Index