DF-3539 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 | --- a/sys/netgraph/async/ng_async.c 2026-09-14 16:57:23.760163755 +0000 +++ b/sys/netgraph/async/ng_async.c 2026-09-14 16:57:41.043944941 +0000 @@ -51,6 +51,7 @@ #include <sys/mbuf.h> #include <sys/malloc.h> #include <sys/errno.h> +#include <sys/spinlock2.h> #include <netgraph/ng_message.h> #include <netgraph/netgraph.h> @@ -66,6 +67,7 @@ /* Private data structure */ struct ng_async_private { + struct spinlock sc_lock; /* serializes cfg/buf/slen vs rcvdata */ node_p node; /* Our node */ hook_p async; /* Asynchronous side */ hook_p sync; /* Synchronous side */ @@ -222,6 +224,7 @@ sc = kmalloc(sizeof(*sc), M_NETGRAPH, M_NOWAIT | M_ZERO); if (sc == NULL) return (ENOMEM); + spin_init(&sc->sc_lock, "ng_async"); sc->amode = MODE_HUNT; sc->cfg.accm = ~0; sc->cfg.amru = NG_ASYNC_DEFAULT_MRU; @@ -305,6 +308,7 @@ struct ng_async_cfg *const cfg = (struct ng_async_cfg *) msg->data; u_char *buf; + u_char *abuf_old = NULL, *sbuf_old = NULL; if (msg->header.arglen != sizeof(*cfg)) ERROUT(EINVAL); @@ -314,20 +318,38 @@ || cfg->smru > NG_ASYNC_MAX_MRU) ERROUT(EINVAL); cfg->enabled = !!cfg->enabled; /* normalize */ + /* + * v1 netgraph runs rcvmsg and rcvdata directly in + * the senders' contexts with no per-node + * serialization, so this realloc/cfg swap races the + * encode/decode loops and can free buffers they + * are still writing into (DF-3539). Serialize all + * mutation of cfg/buffers/slen against the data + * path, which takes the same lock per byte. + */ + spin_lock(&sc->sc_lock); if (cfg->smru > sc->cfg.smru) { /* reallocate buffer */ buf = kmalloc(ASYNC_BUF_SIZE(cfg->smru), M_NETGRAPH, M_NOWAIT); - if (!buf) + if (!buf) { + spin_unlock(&sc->sc_lock); ERROUT(ENOMEM); - kfree(sc->abuf, M_NETGRAPH); + } + abuf_old = sc->abuf; sc->abuf = buf; } if (cfg->amru > sc->cfg.amru) { /* reallocate buffer */ buf = kmalloc(SYNC_BUF_SIZE(cfg->amru), M_NETGRAPH, M_NOWAIT); - if (!buf) + if (!buf) { + spin_unlock(&sc->sc_lock); + if (abuf_old) { + kfree(sc->abuf, M_NETGRAPH); + sc->abuf = abuf_old; + } ERROUT(ENOMEM); - kfree(sc->sbuf, M_NETGRAPH); + } + sbuf_old = sc->sbuf; sc->sbuf = buf; sc->amode = MODE_HUNT; sc->slen = 0; @@ -337,6 +359,11 @@ sc->slen = 0; } sc->cfg = *cfg; + spin_unlock(&sc->sc_lock); + if (abuf_old) + kfree(abuf_old, M_NETGRAPH); + if (sbuf_old) + kfree(sbuf_old, M_NETGRAPH); break; } case NGM_ASYNC_CMD_GET_CONFIG: @@ -417,6 +444,7 @@ nga_async_add(const sc_p sc, u_int16_t *fcs, u_int32_t accm, int *len, u_char x) { *fcs = PPP_FCS(*fcs, x); + spin_lock(&sc->sc_lock); if ((x < 32 && ((1 << x) & accm)) || (x == PPP_ESCAPE) || (x == PPP_FLAG)) { @@ -424,6 +452,7 @@ x ^= PPP_TRANS; } sc->abuf[(*len)++] = x; + spin_unlock(&sc->sc_lock); } /* @@ -480,7 +509,9 @@ /* Add beginning sync flag if it's been long enough to need one */ getmicrotime(&time); if (time.tv_sec >= sc->lasttime + 1) { + spin_lock(&sc->sc_lock); sc->abuf[alen++] = PPP_FLAG; + spin_unlock(&sc->sc_lock); sc->lasttime = time.tv_sec; } @@ -498,7 +529,9 @@ fcs0 = fcs; ADD_BYTE(~fcs0 & 0xff); ADD_BYTE(~fcs0 >> 8); + spin_lock(&sc->sc_lock); sc->abuf[alen++] = PPP_FLAG; + spin_unlock(&sc->sc_lock); /* Put frame in an mbuf and ship it off */ if (!(m = m_devget(sc->abuf, alen, 0, rcvif))) { @@ -591,7 +624,13 @@ continue; } - /* Add byte to frame */ + /* + * The guard and the write must observe a + * consistent (cfg.amru, sbuf, slen) tuple, or a + * concurrent SET_CONFIG swap can corrupt them + * mid-pair (DF-3539 runaway overflow). + */ + spin_lock(&sc->sc_lock); if (sc->slen >= SYNC_BUF_SIZE(sc->cfg.amru)) { sc->stats.asyncOverflows++; sc->amode = MODE_HUNT; @@ -600,6 +639,7 @@ sc->sbuf[sc->slen++] = ch; sc->fcs = PPP_FCS(sc->fcs, ch); } + spin_unlock(&sc->sc_lock); } m = m_free(m); } |