Date: Fri, 17 Apr 2009 10:24:26 -0700 From: Maksim Yevmenkin <maksim.yevmenkin@gmail.com> To: pluknet <pluknet@gmail.com>, Alexander Best <alexbestms@math.uni-muenster.de> Cc: freebsd-current@freebsd.org Subject: Re: possible bug in the sbappendrecord_locked()? (Was: Re: core dump with bluetooth device) Message-ID: <bb4a86c70904171024k255049du8e789511bc35521a@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
On Fri, Apr 17, 2009 at 8:55 AM, Maksim Yevmenkin <maksim.yevmenkin@gmail.com> wrote: > 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: >>>>>>> 0xc8d54d00 >>>>>>> panic: sblastmbufchk from /usr/src/sys/kern/uipc_sockbuf.c:797 >>>>>>> cpuid = 0 >>>>>> >>>>>> are you, by change, have "options SOCKBUF_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 == sb_mbtail == sb_lastrecord == NULL >>>>> >>>>> 2) sbappendrecord_locked() is given mbuf m0 with has exactly one mbuf, >>>>> i.e. m0->m_next == NULL >>>>> >>>>> so >>>>> >>>>> void >>>>> sbappendrecord_locked(struct sockbuf *sb, struct mbuf *m0) >>>>> { >>>>> struct mbuf *m; >>>>> >>>>> SOCKBUF_LOCK_ASSERT(sb); >>>>> >>>>> if (m0 == 0) >>>>> return; >>>>> m = sb->sb_mb; >>>>> >>>>> if (m) >>>>> while (m->m_nextpkt) >>>>> m = m->m_nextpkt; >>>>> >>>>> --> m is still NULL at this point >>>>> >>>>> /* >>>>> * Put the first mbuf on the queue. Note this permits zero length >>>>> * records. >>>>> */ >>>>> sballoc(sb, m0); >>>>> SBLASTRECORDCHK(sb); >>>>> >>>>> --> passed successfully, because sb_mb == sb_lastrecord == NULL (i.e. >>>>> sockbuf is empty) >>>>> >>>>> SBLINKRECORD(sb, m0); >>>>> >>>>> --> at this point sb_mb == sb_lastrecord == m0, _but_ sb_mtail == NULL >>>>> >>>>> if (m) >>>>> m->m_nextpkt = m0; >>>>> else >>>>> sb->sb_mb = m0; >>>>> >>>>> --> not sure about those lines above, didn't SBLINKRECORD(sb, m0) take >>>>> care of it already? >>>>> --> in any case, still, sb_mb == sb_lastrecord == m0 _and_ still >>>>> sb_mtail == NULL >>>>> >>>>> m = m0->m_next; >>>>> m0->m_next = 0; >>>>> >>>>> --> m is still NULL here >>>>> >>>>> if (m && (m0->m_flags & M_EOR)) { >>>>> m0->m_flags &= ~M_EOR; >>>>> m->m_flags |= M_EOR; >>>>> } >>>>> >>>>> --> sbcompress() is called with m == NULL, which is triggers the panic >>>>> (read below) >>>>> >>>>> sbcompress(sb, m, m0); >>>>> } >>>>> >>>>> =========== >>>>> >>>>> void >>>>> sbcompress(struct sockbuf *sb, struct mbuf *m, struct mbuf *n) >>>>> { >>>>> int eor = 0; >>>>> struct mbuf *o; >>>>> >>>>> SOCKBUF_LOCK_ASSERT(sb); >>>>> >>>>> while (m) { >>>>> >>>>> --> lots of code that never gets executed because m == NULL >>>>> >>>>> } >>>>> if (eor) { >>>>> KASSERT(n != NULL, ("sbcompress: eor && n == NULL")); >>>>> n->m_flags |= eor; >>>>> } >>>>> >>>>> --> this where panic happens, because sb_mbtail is still NULL, but >>>>> sockbuf now contains exactly one record >>>>> >>>>> SBLASTMBUFCHK(sb); >>>>> } >>>>> >>>>> so, it looks like, sbcompress() should only be called when m != NULL. >>>>> also, when m == 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 work? am i missing something? > >> svn diff > Index: uipc_sockbuf.c > =================================================================== > --- uipc_sockbuf.c (revision 191012) > +++ uipc_sockbuf.c (working copy) > @@ -577,10 +577,6 @@ > > if (m0 == 0) > return; > - m = sb->sb_mb; > - if (m) > - while (m->m_nextpkt) > - m = m->m_nextpkt; > /* > * Put the first mbuf on the queue. Note this permits zero length > * records. > @@ -588,17 +584,17 @@ > sballoc(sb, m0); > SBLASTRECORDCHK(sb); > SBLINKRECORD(sb, m0); > - if (m) > - m->m_nextpkt = m0; > - else > - sb->sb_mb = m0; > + sb->sb_mbtail = m0; > m = m0->m_next; > m0->m_next = 0; > - if (m && (m0->m_flags & M_EOR)) { > - m0->m_flags &= ~M_EOR; > - m->m_flags |= M_EOR; > + if (m != NULL) { > + if (m0->m_flags & M_EOR) { > + m0->m_flags &= ~M_EOR; > + m->m_flags |= M_EOR; > + } > + > + sbcompress(sb, m, m0); > } > - sbcompress(sb, m, m0); > } > > /* > > == the patch seems to work for me. i do not see any crashes. please try it and let me know if you still have crashes with SOCKBUF_DEBUG thanks, max
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bb4a86c70904171024k255049du8e789511bc35521a>