From owner-freebsd-hackers@FreeBSD.ORG Fri Jan 11 08:13:03 2013 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 2B65184A for ; Fri, 11 Jan 2013 08:13:03 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.69.10]) by mx1.freebsd.org (Postfix) with ESMTP id 76BD58BA for ; Fri, 11 Jan 2013 08:13:01 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.5/8.14.5) with ESMTP id r0B8CsQY046496; Fri, 11 Jan 2013 12:12:54 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.5/8.14.5/Submit) id r0B8CsK9046495; Fri, 11 Jan 2013 12:12:54 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Fri, 11 Jan 2013 12:12:54 +0400 From: Gleb Smirnoff To: Jacques Fourie Subject: Re: Possible bug in m_split() when splitting M_EXT mbufs Message-ID: <20130111081254.GG82815@FreeBSD.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Hackers freeBSD X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Jan 2013 08:13:03 -0000 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.