Well, the m_copyback() may need to allocate new mbufs in order to hold the data, so the allocation may fail because it uses the 'M_NOWAIT' flag and doesn't provided any parameter to use the 'M_WAITOK' flag for allocation. (For the reference, OpenBSD extended m_copyback() to have the extra 'how' parameter for this purpose.) As a consequence, the caller should check the resulting packet length for m_copyback() failure. Below is the patch to 'if_wg.c' function 'wg_send_buf()', please review. --- if_wg.c.orig 2023-10-12 09:06:16.983637264 +0800 +++ if_wg.c 2023-10-21 15:29:47.928807521 +0800 @@ -911,6 +911,11 @@ retry: goto out; } m_copyback(m, 0, len, buf); + if (m->m_pkthdr.len != len) { + m_freem(m); + ret = ENOMEM; + goto out; + } if (ret == 0) { ret = wg_send(sc, e, m);
(In reply to Aaron LI from comment #0) I don't think that's an issue here because the mbuf is known to be of length 'len' before it's passed to m_copyout(). The allocation is done just before the m_copyback().
(In reply to Kristof Provost from comment #1) Might be worth adding a KASSERT for, though I imagine if something is wrong with the accounting in m_copyback() we would've seen it elsewhere.
(In reply to Kristof Provost from comment #1) > I don't think that's an issue here because the mbuf is known to be of > length 'len' before it's passed to m_copyout(). The allocation is done > just before the m_copyback(). Oh, that's true. I forget that implication set by m_get2() just before m_copyback(). As kevans suggested, a KASSERT() after m_copyback() would make it clear and remove this confusion.
Hmm, you could perhaps KASSERT that mget2() returned a chain of the proper length, but that's really just asserting that the API works as documented. I'm not quite sure it's worth asserting after m_copyback as the length really shouldn't change. Other places in the tree that use the pattern of preallocating the mbuf chain by m_getm or m_get2 don't check the length after m_copyback (linux_80211.c, iw_cxgbe/cm.c, gve_rx.c were ones I looked at), so I don't think it's worth adding an assertion here.