2009/4/17 Maksim Yevmenkin <maksim.yevmenkin_at_gmail.com>: > On Thu, Apr 16, 2009 at 9:48 PM, pluknet <pluknet_at_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_at_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? I'm on vacations and will not able to test it until after 4/23. :( > >> svn diff > Index: uipc_sockbuf.c > =================================================================== > --- uipc_sockbuf.c (revision 191012) > +++ uipc_sockbuf.c (working copy) > _at__at_ -577,10 +577,6 _at__at_ > > 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. > _at__at_ -588,17 +584,17 _at__at_ > 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); > } > > /* > > == > > thanks, > max > -- wbr, pluknetReceived on Sat Apr 18 2009 - 12:41:43 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:46 UTC