Date: Wed, 21 Oct 2009 01:10:06 -0700 From: Qing Li <qingli@freebsd.org> To: Robert Watson <rwatson@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r198301 - head/sys/netinet Message-ID: <9ace436c0910210110gc359cd1jea3dbd49db7e8733@mail.gmail.com> In-Reply-To: <alpine.BSF.2.00.0910210853540.24137@fledge.watson.org> References: <200910201755.n9KHtgT9073966@svn.freebsd.org> <alpine.BSF.2.00.0910210853540.24137@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
I believe this patch will fix your crash.
-- Qing
On Wed, Oct 21, 2009 at 12:58 AM, Robert Watson <rwatson@freebsd.org> wrote=
:
> On Tue, 20 Oct 2009, Qing Li wrote:
>
>> =A0In the ARP callout timer expiration function, the current time_second
>> =A0is compared against the entry expiration time value (that was set bas=
ed
>> =A0on time_second) to check if the current time is larger than the set
>> =A0expiration time. Due to the +/- timer granularity value, the comparis=
on
>> =A0returns false, causing the alternative code to be executed. The
>> =A0alternative code path freed the memory without removing that entry
>> =A0from the table list, causing a use-after-free bug.
>>
>> =A0Reviewed by: =A0 discussed with kmacy
>> =A0MFC after: =A0 =A0 immediately
>> =A0Verified by: =A0 rnoland, yongari
>
> Could this be the same problem I ran into overnight on an 18 October kern=
el:
>
> Fatal trap 12: page fault while in kernel mode
> cpuid =3D 0; apic id =3D 00
> fault virtual address =A0 =3D 0x7572749f
> fault code =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D supervisor read, page not prese=
nt
> instruction pointer =A0 =A0 =3D 0x20:0xc09c0551
> stack pointer =A0 =A0 =A0 =A0 =A0 =3D 0x28:0xc43f6ab4
> frame pointer =A0 =A0 =A0 =A0 =A0 =3D 0x28:0xc43f6adc
> code segment =A0 =A0 =A0 =A0 =A0 =A0=3D base 0x0, limit 0xfffff, type 0x1=
b
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D DPL 0, pres 1, def32 1=
, gran 1
> processor eflags =A0 =A0 =A0 =A0=3D interrupt enabled, resume, IOPL =3D 0
> current process =A0 =A0 =A0 =A0 =3D 0 (em0 taskq)
>
> #9 =A00xc0be731b in calltrap () at ../../../i386/i386/exception.s:165
> #10 0xc09c0551 in in_lltable_lookup (llt=3D0xc4955200, flags=3D8192,
> =A0 =A0l3addr=3D0xc43f6b60) at ../../../netinet/in.c:1361
> #11 0xc09b8ea7 in arpintr (m=3D0xc48dcd00) at if_llatbl.h:202
> #12 0xc096f899 in netisr_dispatch_src (proto=3D7, source=3D0, m=3D0xc48dc=
d00)
> =A0 =A0at ../../../net/netisr.c:917
> #13 0xc096fb30 in netisr_dispatch (proto=3D7, m=3D0xc48dcd00)
> =A0 =A0at ../../../net/netisr.c:1004
> #14 0xc0967c11 in ether_demux (ifp=3D0xc470b400, m=3D0xc48dcd00)
> =A0 =A0at ../../../net/if_ethersubr.c:895
> #15 0xc0968163 in ether_input (ifp=3D0xc470b400, m=3D0xc48dcd00)
> =A0 =A0at ../../../net/if_ethersubr.c:754
> #16 0xc063bcaa in em_rxeof (adapter=3D0xc470d000, count=3D99)
> =A0 =A0at ../../../dev/e1000/if_em.c:4610
> #17 0xc063dfc7 in em_handle_rxtx (context=3D0xc470d000, pending=3D1)
> =A0 =A0at ../../../dev/e1000/if_em.c:1763
>
> (kgdb) frame 10
> #10 0xc09c0551 in in_lltable_lookup (llt=3D0xc4955200, flags=3D8192,
> =A0 =A0l3addr=3D0xc43f6b60) at ../../../netinet/in.c:1361
> 1361 =A0 =A0 =A0 =A0 =A0 =A0LIST_FOREACH(lle, lleh, lle_next) {
> (kgdb) list
> 1356 =A0 =A0 =A0 =A0 =A0 =A0KASSERT(l3addr->sa_family =3D=3D AF_INET,
> 1357 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0("sin_family %d", l3addr->sa_family))=
;
> 1358
> 1359 =A0 =A0 =A0 =A0 =A0 =A0hashkey =3D sin->sin_addr.s_addr;
> 1360 =A0 =A0 =A0 =A0 =A0 =A0lleh =3D &llt->lle_head[LLATBL_HASH(hashkey, =
LLTBL_HASHMASK)];
> 1361 =A0 =A0 =A0 =A0 =A0 =A0LIST_FOREACH(lle, lleh, lle_next) {
> 1362 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct sockaddr_in *sa2 =3D (=
struct sockaddr_in
> *)L3_ADDR(lle);
> 1363 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (lle->la_flags & LLE_DELET=
ED)
> 1364 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0continue;
> 1365 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (sa2->sin_addr.s_addr =3D=
=3D sin->sin_addr.s_addr)
> (kgdb) print *llt
> $5 =3D {llt_link =3D {sle_next =3D 0x0}, lle_head =3D {{lh_first =3D 0xc7=
60d580}, {
> =A0 =A0 =A0lh_first =3D 0x0}, {lh_first =3D 0x0}, {lh_first =3D 0xc75c390=
0}, {
> =A0 =A0 =A0lh_first =3D 0x0} <repeats 14 times>, {lh_first =3D 0xc49a8380=
}, {
> =A0 =A0 =A0lh_first =3D 0x0}, {lh_first =3D 0x0}, {lh_first =3D 0x0}, {lh=
_first =3D 0x0},
> {
> =A0 =A0 =A0lh_first =3D 0x0}, {lh_first =3D 0x0}, {lh_first =3D 0x0}, {lh=
_first =3D 0x0},
> {
> =A0 =A0 =A0lh_first =3D 0x0}, {lh_first =3D 0xc61be780}, {lh_first =3D 0x=
0}, {
> =A0 =A0 =A0lh_first =3D 0x0}, {lh_first =3D 0x0}}, llt_af =3D 2, llt_ifp =
=3D 0xc470b400,
> =A0llt_new =3D 0xc09bdd60 <in_lltable_new>,
> =A0llt_free =3D 0xc09c0490 <in_lltable_free>,
> =A0llt_prefix_free =3D 0xc09c09b0 <in_lltable_prefix_free>,
> =A0llt_lookup =3D 0xc09c0510 <in_lltable_lookup>,
> =A0llt_rtcheck =3D 0xc09bdbe0 <in_lltable_rtcheck>,
> =A0llt_dump =3D 0xc09bd9c0 <in_lltable_dump>}
> (kgdb) print *l3addr
> $6 =3D {sa_len =3D 16 '\020', sa_family =3D 2 '\002',
> =A0sa_data =3D "\000\000??*\001\000\000\000\000\000\000\000"}
> (kgdb) print lle
> $7 =3D (struct llentry *) 0x75727473
> (kgdb) print lleh
> $8 =3D (struct llentries *) 0xc4955210
> (kgdb) print *lleh
> $9 =3D {lh_first =3D 0xc75c3900}
>
> Your commit was after this kernel, so I'd be quite happy with the answer
> "now fixed" but wanted to be sure.
>
> Robert
>
>>
>> Modified:
>> =A0head/sys/netinet/if_ether.c
>>
>> Modified: head/sys/netinet/if_ether.c
>>
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> --- head/sys/netinet/if_ether.c Tue Oct 20 17:50:36 2009 =A0 =A0 =A0 =A0=
(r198300)
>> +++ head/sys/netinet/if_ether.c Tue Oct 20 17:55:42 2009 =A0 =A0 =A0 =A0=
(r198301)
>> @@ -175,18 +175,18 @@ arptimer(void *arg)
>> =A0 =A0 =A0 =A0CURVNET_SET(ifp->if_vnet);
>> =A0 =A0 =A0 =A0IF_AFDATA_LOCK(ifp);
>> =A0 =A0 =A0 =A0LLE_WLOCK(lle);
>> - =A0 =A0 =A0 if (((lle->la_flags & LLE_DELETED) ||
>> - =A0 =A0 =A0 =A0 =A0 (time_second >=3D lle->la_expire)) &&
>> - =A0 =A0 =A0 =A0 =A0 (!callout_pending(&lle->la_timer) &&
>> + =A0 =A0 =A0 if ((!callout_pending(&lle->la_timer) &&
>> =A0 =A0 =A0 =A0 =A0 =A0callout_active(&lle->la_timer))) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(void) llentry_free(lle);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ARPSTAT_INC(timeouts);
>> - =A0 =A0 =A0 } else {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 /*
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* Still valid, just drop our reference
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 LLE_FREE_LOCKED(lle);
>> + =A0 =A0 =A0 }
>> +#ifdef DIAGNOSTICS
>> + =A0 =A0 =A0 else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct sockaddr *l3addr =3D L3_ADDR(lle);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 log(LOG_INFO, "arptimer issue: %p, IPv4 ad=
dress:
>> \"%s\"\n", lle,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 inet_ntoa(((const struct sockaddr_=
in
>> *)l3addr)->sin_addr));
>> =A0 =A0 =A0 =A0}
>> +#endif
>> =A0 =A0 =A0 =A0IF_AFDATA_UNLOCK(ifp);
>> =A0 =A0 =A0 =A0CURVNET_RESTORE();
>> }
>> @@ -377,7 +377,7 @@ retry:
>>
>> =A0 =A0 =A0 =A0if (renew) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0LLE_ADDREF(la);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 la->la_expire =3D time_second;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 la->la_expire =3D time_second + V_arpt_dow=
n;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0callout_reset(&la->la_timer, hz * V_arpt_=
down, arptimer,
>> la);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0la->la_asked++;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0LLE_WUNLOCK(la);
>>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9ace436c0910210110gc359cd1jea3dbd49db7e8733>