possible bug in the sbappendrecord_locked()? (Was: Re: core
dump with bluetooth device)
Maksim Yevmenkin
maksim.yevmenkin at gmail.com
Fri Apr 17 02:45:05 UTC 2009
On Thu, Apr 16, 2009 at 7:41 PM, Maksim Yevmenkin
<maksim.yevmenkin at gmail.com> wrote:
> On Thu, Apr 16, 2009 at 7:22 PM, Maksim Yevmenkin
> <maksim.yevmenkin at gmail.com> wrote:
>> On Thu, Apr 16, 2009 at 5:39 PM, Maksim Yevmenkin
>> <maksim.yevmenkin at gmail.com> wrote:
>>> On Thu, Apr 16, 2009 at 12:59 PM, Alexander Best
>>> <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?
>
> i think there is also something strange going on in
> sbappendaddr_locked(), basically,
>
> int
> sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa,
> struct mbuf *m0, struct mbuf *control)
> {
> struct mbuf *m, *n, *nlast;
> int space = asa->sa_len;
>
> SOCKBUF_LOCK_ASSERT(sb);
>
> if (m0 && (m0->m_flags & M_PKTHDR) == 0)
> panic("sbappendaddr_locked");
> if (m0)
> space += m0->m_pkthdr.len;
> space += m_length(control, &n);
>
> if (space > sbspace(sb))
> return (0);
> #if MSIZE <= 256
> if (asa->sa_len > MLEN)
> return (0);
> #endif
> MGET(m, M_DONTWAIT, MT_SONAME);
> if (m == 0)
> return (0);
> m->m_len = asa->sa_len;
> bcopy(asa, mtod(m, caddr_t), asa->sa_len);
>
> --> at this point n is not initialized, or at least i do not see where
> it was initialized. shouldn't be compiler giving a warning here?
>
> if (n)
> n->m_next = m0; /* concatenate data to control */
> else
> control = m0;
>
> am i missing something obvious here?
ignore the last one :) i missed m_length(control, &n). ok, need to get
away from the screen :)
thanks,
max
More information about the freebsd-current
mailing list