Date: Thu, 16 Apr 2009 19:41:02 -0700 From: Maksim Yevmenkin <maksim.yevmenkin@gmail.com> To: freebsd-current@freebsd.org Cc: Alexander Best <alexbestms@math.uni-muenster.de> Subject: Re: possible bug in the sbappendrecord_locked()? (Was: Re: core dump with bluetooth device) Message-ID: <bb4a86c70904161941v53cc0f90i8a1c94b1c0458e61@mail.gmail.com> In-Reply-To: <bb4a86c70904161922t38819fd8r839e5e832aa1f1@mail.gmail.com> References: <bb4a86c70904161922t38819fd8r839e5e832aa1f1@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Apr 16, 2009 at 7:22 PM, Maksim Yevmenkin
<maksim.yevmenkin@gmail.com> wrote:
> On Thu, Apr 16, 2009 at 5:39 PM, Maksim Yevmenkin
> <maksim.yevmenkin@gmail.com> wrote:
>> On Thu, Apr 16, 2009 at 12:59 PM, Alexander Best
>> <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_lastrecord =
=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 permits z=
ero 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 NULL (=
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_mtail=
=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) take
> care of it already?
> --> in any case, still, sb_mb =3D=3D sb_lastrecord =3D=3D m0 _and_ still
> 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 pani=
c
> (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 NULL.
> also, when m =3D=3D 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 =3D asa->sa_len;
SOCKBUF_LOCK_ASSERT(sb);
if (m0 && (m0->m_flags & M_PKTHDR) =3D=3D 0)
panic("sbappendaddr_locked");
if (m0)
space +=3D m0->m_pkthdr.len;
space +=3D m_length(control, &n);
if (space > sbspace(sb))
return (0);
#if MSIZE <=3D 256
if (asa->sa_len > MLEN)
return (0);
#endif
MGET(m, M_DONTWAIT, MT_SONAME);
if (m =3D=3D 0)
return (0);
m->m_len =3D 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 =3D m0; /* concatenate data to control */
else
control =3D m0;
am i missing something obvious here?
thanks
max
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bb4a86c70904161941v53cc0f90i8a1c94b1c0458e61>