Possible bug in m_split() when splitting M_EXT mbufs

[Gleb Smirnoff]

  Jacques,

On Fri, Jan 11, 2013 at 09:34:32AM +0200, Jacques Fourie wrote:
J> Could someone please verify if m_split as in svn rev 245286 is doing the
J> right thing in the scenario where a mbuf chain is split with len0 falling
J> on a mbuf boundary and the mbuf in question being a M_EXT mbuf? Consider
J> the following example where m0 is a mbuf chain consisting of 2 M_EXT mbufs,
J> both 1448 bytes in length. Let len0 be 1448. The 'len0 > m->m_len' check
J> will be false so the for loop will not be entered in this case. We now have
J> len = 1448 and remain = 0 and m still points to the first mbuf in the
J> chain. Also assume that m0 is a pkthdr mbuf. A new pkthdr mbuf n will be
J> allocated and initialized before the following piece of code is executed :
J> 
J> extpacket:
J>         if (m->m_flags & M_EXT) {
J>                 n->m_data = m->m_data + len;
J>                 mb_dupcl(n, m);
J>         } else {
J>                 bcopy(mtod(m, caddr_t) + len, mtod(n, caddr_t), remain);
J>         }
J>         n->m_len = remain;
J>         m->m_len = len;
J>         n->m_next = m->m_next;
J>         m->m_next = NULL;
J>         return (n);
J> 
J> As m is a M_EXT mbuf the code in the if() clause will be executed. The
J> problem is that m still points to the first mbuf so effectively the data
J> pointer for n is assigned to the end of m's data pointer. It should
J> actually point to the start of the data pointer of the next mbuf in the
J> original m0 chain, right?

Thanks for analysis, Jacques.

IMHO, the following patch should suffice and won't break anything:

Index: uipc_mbuf.c
===================================================================
--- uipc_mbuf.c (revision 245223)
+++ uipc_mbuf.c (working copy)
@@ -1126,7 +1126,7 @@
        u_int len = len0, remain;
 
        MBUF_CHECKSLEEP(wait);
-       for (m = m0; m && len > m->m_len; m = m->m_next)
+       for (m = m0; m && len >= m->m_len; m = m->m_next)
                len -= m->m_len;
        if (m == NULL)
                return (NULL);

Can you please test it?

-- 
Totus tuus, Glebius.