Date: Fri, 11 Jan 2013 12:12:54 +0400 From: Gleb Smirnoff <glebius@FreeBSD.org> To: Jacques Fourie <jacques.fourie@gmail.com> Cc: Hackers freeBSD <freebsd-hackers@FreeBSD.org> Subject: Re: Possible bug in m_split() when splitting M_EXT mbufs Message-ID: <20130111081254.GG82815@FreeBSD.org> In-Reply-To: <CALX0vxAhRz--NTG1yLHTf1xQxkiqixyWEndeYnuxgsTzctq5-g@mail.gmail.com> References: <CALX0vxAhRz--NTG1yLHTf1xQxkiqixyWEndeYnuxgsTzctq5-g@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130111081254.GG82815>