From owner-freebsd-current@FreeBSD.ORG Sat Apr 18 14:41:43 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9E90F1065807 for ; Sat, 18 Apr 2009 14:41:43 +0000 (UTC) (envelope-from pluknet@gmail.com) Received: from mu-out-0910.google.com (mu-out-0910.google.com [209.85.134.184]) by mx1.freebsd.org (Postfix) with ESMTP id 1ED338FC14 for ; Sat, 18 Apr 2009 14:41:42 +0000 (UTC) (envelope-from pluknet@gmail.com) Received: by mu-out-0910.google.com with SMTP id w9so487613mue.3 for ; Sat, 18 Apr 2009 07:41:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=FwBFEdrepABQqBwkNljCEOW9QopvC8zUdhK7Nr4YW9A=; b=g04jXW//PLb7wwBGl7ukpnNXy0TIBElKQj2NXaASk70a9HxV1QkqJugKdXlJY+jO/S pBRBJJYs+l/2xWMxQ/HJeVGrYZ3c546grtN+xooWupRJpk6c+JyZ7iH6Jba5PjrSYX9L KeoLpQNRKDCrHAE+upyLjMGX9b18zYmp5jxmw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=fRUpzZVtzd6VcnCc5yNgd/+riJtR1mFRrwCzwSOQtnJBfMtA5x9CNDskTVxqHa0XtY /doB/0zu9hPI9ROhzJxmxPMo33SvL2t24xpy89m5sZxBD7/c300AlS7SHpOHNbkLgjex fkcDKao2r6fhwXnznAVDlLcnJBz3Z8kApVgb8= MIME-Version: 1.0 Received: by 10.103.171.6 with SMTP id y6mr2033741muo.31.1240065702040; Sat, 18 Apr 2009 07:41:42 -0700 (PDT) In-Reply-To: References: Date: Sat, 18 Apr 2009 07:41:41 -0700 Message-ID: From: pluknet To: Maksim Yevmenkin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Alexander Best , freebsd-current@freebsd.org Subject: Re: possible bug in the sbappendrecord_locked()? (Was: Re: core dump with bluetooth device) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 Apr 2009 14:41:47 -0000 2009/4/17 Maksim Yevmenkin : > On Thu, Apr 16, 2009 at 9:48 PM, pluknet wrote: >> Sorry for top-posting. >> >> This is a fairly old bug. >> See my investigation >> http://lists.freebsd.org/pipermail/freebsd-net/2008-August/019345.html > > ahh, i see. thanks for the pointer. please read below. > >>>>>> wrote: >>>>>>> hi there, >>>>>>> >>>>>>> i'm running r191076M. when i try to send files from my mobile phone= to my >>>>>>> computer via bt the core dumps. here's a backtrace: >>>>>>> >>>>>>> Unread portion of the kernel message buffer: >>>>>>> sblastmbufchk: sb_mb 0xc8d54d00 sb_mbtail 0 last 0xc8d54d00 >>>>>>> packet tree: >>>>>>> =A0 =A0 =A0 =A00xc8d54d00 >>>>>>> panic: sblastmbufchk from /usr/src/sys/kern/uipc_sockbuf.c:797 >>>>>>> cpuid =3D 0 >>>>>> >>>>>> are you, by change, have "options =A0SOCKBUF_DEBUG" in your kernel? >>>>> >>>>> ok, there is something strange going on in the >>>>> sbappendrecord_locked(). consider the following initial conditions: >>>>> >>>>> 1) sockbuf sb is empty, i.e. sb_mb =3D=3D sb_mbtail =3D=3D sb_lastrec= ord =3D=3D NULL >>>>> >>>>> 2) sbappendrecord_locked() is given mbuf m0 with has exactly one mbuf= , >>>>> i.e. m0->m_next =3D=3D NULL >>>>> >>>>> so >>>>> >>>>> void >>>>> sbappendrecord_locked(struct sockbuf *sb, struct mbuf *m0) >>>>> { >>>>> =A0 =A0 =A0 =A0struct mbuf *m; >>>>> >>>>> =A0 =A0 =A0 =A0SOCKBUF_LOCK_ASSERT(sb); >>>>> >>>>> =A0 =A0 =A0 =A0if (m0 =3D=3D 0) >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return; >>>>> =A0 =A0 =A0 =A0m =3D sb->sb_mb; >>>>> >>>>> =A0 =A0 =A0 =A0if (m) >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0while (m->m_nextpkt) >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0m =3D m->m_nextpkt; >>>>> >>>>> --> m is still NULL at this point >>>>> >>>>> =A0 =A0 =A0 =A0/* >>>>> =A0 =A0 =A0 =A0 * Put the first mbuf on the queue. =A0Note this permi= ts zero length >>>>> =A0 =A0 =A0 =A0 * records. >>>>> =A0 =A0 =A0 =A0 */ >>>>> =A0 =A0 =A0 =A0sballoc(sb, m0); >>>>> =A0 =A0 =A0 =A0SBLASTRECORDCHK(sb); >>>>> >>>>> --> passed successfully, because sb_mb =3D=3D sb_lastrecord =3D=3D NU= LL (i.e. >>>>> sockbuf is empty) >>>>> >>>>> =A0 =A0 =A0 =A0SBLINKRECORD(sb, m0); >>>>> >>>>> --> at this point sb_mb =3D=3D sb_lastrecord =A0=3D=3D m0, _but_ sb_m= tail =3D=3D NULL >>>>> >>>>> =A0 =A0 =A0 =A0if (m) >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0m->m_nextpkt =3D m0; >>>>> =A0 =A0 =A0 =A0else >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sb->sb_mb =3D m0; >>>>> >>>>> --> not sure about those lines above, didn't SBLINKRECORD(sb, m0) tak= e >>>>> care of it already? >>>>> --> in any case, still, sb_mb =3D=3D sb_lastrecord =3D=3D m0 _and_ st= ill >>>>> sb_mtail =3D=3D NULL >>>>> >>>>> =A0 =A0 =A0 =A0m =3D m0->m_next; >>>>> =A0 =A0 =A0 =A0m0->m_next =3D 0; >>>>> >>>>> --> m is still NULL here >>>>> >>>>> =A0 =A0 =A0 =A0if (m && (m0->m_flags & M_EOR)) { >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0m0->m_flags &=3D ~M_EOR; >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0m->m_flags |=3D M_EOR; >>>>> =A0 =A0 =A0 =A0} >>>>> >>>>> --> sbcompress() is called with m =3D=3D NULL, which is triggers the = panic >>>>> (read below) >>>>> >>>>> =A0 =A0 =A0 =A0sbcompress(sb, m, m0); >>>>> } >>>>> >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>> >>>>> void >>>>> sbcompress(struct sockbuf *sb, struct mbuf *m, struct mbuf *n) >>>>> { >>>>> =A0 =A0 =A0 =A0int eor =3D 0; >>>>> =A0 =A0 =A0 =A0struct mbuf *o; >>>>> >>>>> =A0 =A0 =A0 =A0SOCKBUF_LOCK_ASSERT(sb); >>>>> >>>>> =A0 =A0 =A0 =A0while (m) { >>>>> >>>>> --> lots of code that never gets executed because m =3D=3D NULL >>>>> >>>>> =A0 =A0 =A0 =A0} >>>>> =A0 =A0 =A0 =A0if (eor) { >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0KASSERT(n !=3D NULL, ("sbcompress: eor= && n =3D=3D NULL")); >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0n->m_flags |=3D eor; >>>>> =A0 =A0 =A0 =A0} >>>>> >>>>> --> this where panic happens, because sb_mbtail is still NULL, but >>>>> sockbuf now contains exactly one record >>>>> >>>>> =A0 =A0 =A0 =A0SBLASTMBUFCHK(sb); >>>>> } >>>>> >>>>> so, it looks like, sbcompress() should only be called when m !=3D NUL= L. >>>>> also, when m =3D=3D NULL, m0 should be marked as EOR. >>>> >>>> actually, no, EOR should be set (or not set already). >>>> >>>>> comments anyone? > > > ok, this is completely untested, so be warned :) would something like > the following =A0work? am i missing something? I'm on vacations and will not able to test it until after 4/23. :( > >> svn diff > Index: uipc_sockbuf.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- uipc_sockbuf.c =A0 =A0 =A0(revision 191012) > +++ uipc_sockbuf.c =A0 =A0 =A0(working copy) > @@ -577,10 +577,6 @@ > > =A0 =A0 =A0 =A0if (m0 =3D=3D 0) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return; > - =A0 =A0 =A0 m =3D sb->sb_mb; > - =A0 =A0 =A0 if (m) > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 while (m->m_nextpkt) > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 m =3D m->m_nextpkt; > =A0 =A0 =A0 =A0/* > =A0 =A0 =A0 =A0 * Put the first mbuf on the queue. =A0Note this permits z= ero length > =A0 =A0 =A0 =A0 * records. > @@ -588,17 +584,17 @@ > =A0 =A0 =A0 =A0sballoc(sb, m0); > =A0 =A0 =A0 =A0SBLASTRECORDCHK(sb); > =A0 =A0 =A0 =A0SBLINKRECORD(sb, m0); > - =A0 =A0 =A0 if (m) > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 m->m_nextpkt =3D m0; > - =A0 =A0 =A0 else > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 sb->sb_mb =3D m0; > + =A0 =A0 =A0 sb->sb_mbtail =3D m0; > =A0 =A0 =A0 =A0m =3D m0->m_next; > =A0 =A0 =A0 =A0m0->m_next =3D 0; > - =A0 =A0 =A0 if (m && (m0->m_flags & M_EOR)) { > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 m0->m_flags &=3D ~M_EOR; > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 m->m_flags |=3D M_EOR; > + =A0 =A0 =A0 if (m !=3D NULL) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (m0->m_flags & M_EOR) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 m0->m_flags &=3D ~M_EOR; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 m->m_flags |=3D M_EOR; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sbcompress(sb, m, m0); > =A0 =A0 =A0 =A0} > - =A0 =A0 =A0 sbcompress(sb, m, m0); > =A0} > > =A0/* > > =3D=3D > > thanks, > max > --=20 wbr, pluknet