From owner-freebsd-current@FreeBSD.ORG Fri Apr 17 02:41:04 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 068D41065673 for ; Fri, 17 Apr 2009 02:41:04 +0000 (UTC) (envelope-from maksim.yevmenkin@gmail.com) Received: from yx-out-2324.google.com (yx-out-2324.google.com [74.125.44.29]) by mx1.freebsd.org (Postfix) with ESMTP id B0A148FC20 for ; Fri, 17 Apr 2009 02:41:03 +0000 (UTC) (envelope-from maksim.yevmenkin@gmail.com) Received: by yx-out-2324.google.com with SMTP id 8so442820yxm.13 for ; Thu, 16 Apr 2009 19:41:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=/etj/X+t1/a8ja+IQReP5ZkmbcnQCWOjloECQPSqV7k=; b=IWlWQHHMoMnyHXU3kK/eOOIUSGEYKt9ZqFZgvEmNp2Qfd1cn+pzdn8TQJNREgz5J8D SCU3tQnM5q72aTjK8WxPFshAlwLAiGJPuYOLZwVNfhv+m2d3AVRXnqB971TXD7hFsIKW BAPxjPZh7BmLE+hiA0XPeyWbvijcVD31x4vw0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=rcN4o53auXtLUPy+84EJ9dgEMGVR02j5GaNPiLThElTD1a4o+4JXUC02+OrpgSybf6 7O+dZVMtbYJ9D5zxeHrTlggquo5dq0W9bUUyjaZjRClKeD+kFO0gn/Gzqkc7gpaIY924 rMWTyUsX39r3TM/Wg5dsOA4sBXALlCDV9ca4o= MIME-Version: 1.0 Received: by 10.90.105.6 with SMTP id d6mr2549298agc.1.1239936063012; Thu, 16 Apr 2009 19:41:03 -0700 (PDT) In-Reply-To: References: Date: Thu, 16 Apr 2009 19:41:02 -0700 Message-ID: From: Maksim Yevmenkin To: freebsd-current@freebsd.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Alexander Best Subject: Re: possible bug in the sbappendrecord_locked()? (Was: Re: core dump with bluetooth device) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Apr 2009 02:41:04 -0000 On Thu, Apr 16, 2009 at 7:22 PM, Maksim Yevmenkin wrote: > On Thu, Apr 16, 2009 at 5:39 PM, Maksim Yevmenkin > wrote: >> On Thu, Apr 16, 2009 at 12:59 PM, Alexander Best >> 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