Date: Sat, 18 Apr 2009 07:41:41 -0700 From: pluknet <pluknet@gmail.com> To: Maksim Yevmenkin <maksim.yevmenkin@gmail.com> Cc: Alexander Best <alexbestms@math.uni-muenster.de>, freebsd-current@freebsd.org Subject: Re: possible bug in the sbappendrecord_locked()? (Was: Re: core dump with bluetooth device) Message-ID: <a31046fc0904180741x5ddf9cf6r4991cf7f017faf7b@mail.gmail.com> In-Reply-To: <bb4a86c70904170855u7162ec56x51fa09f6941a156c@mail.gmail.com> References: <bb4a86c70904161922t38819fd8r839e5e832aa1f1@mail.gmail.com> <bb4a86c70904161941v53cc0f90i8a1c94b1c0458e61@mail.gmail.com> <bb4a86c70904161945g32ab6e44nae447d027293733d@mail.gmail.com> <a31046fc0904162148y4c783a99w881100b9553c28ec@mail.gmail.com> <bb4a86c70904170855u7162ec56x51fa09f6941a156c@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
2009/4/17 Maksim Yevmenkin <maksim.yevmenkin@gmail.com>: > On Thu, Apr 16, 2009 at 9:48 PM, pluknet <pluknet@gmail.com> 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. > >>>>>> <alexbestms@math.uni-muenster.de> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?a31046fc0904180741x5ddf9cf6r4991cf7f017faf7b>