From owner-svn-src-all@FreeBSD.ORG Wed Oct 21 07:58:21 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 292E61065676; Wed, 21 Oct 2009 07:58:21 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id F18DF8FC1F; Wed, 21 Oct 2009 07:58:20 +0000 (UTC) Received: from fledge.watson.org (fledge.watson.org [65.122.17.41]) by cyrus.watson.org (Postfix) with ESMTPS id 0EB1546B17; Wed, 21 Oct 2009 03:58:20 -0400 (EDT) Date: Wed, 21 Oct 2009 08:58:19 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Qing Li In-Reply-To: <200910201755.n9KHtgT9073966@svn.freebsd.org> Message-ID: References: <200910201755.n9KHtgT9073966@svn.freebsd.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r198301 - head/sys/netinet X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Oct 2009 07:58:21 -0000 On Tue, 20 Oct 2009, Qing Li wrote: > In the ARP callout timer expiration function, the current time_second > is compared against the entry expiration time value (that was set based > on time_second) to check if the current time is larger than the set > expiration time. Due to the +/- timer granularity value, the comparison > returns false, causing the alternative code to be executed. The > alternative code path freed the memory without removing that entry > from the table list, causing a use-after-free bug. > > Reviewed by: discussed with kmacy > MFC after: immediately > Verified by: rnoland, yongari Could this be the same problem I ran into overnight on an 18 October kernel: Fatal trap 12: page fault while in kernel mode cpuid = 0; apic id = 00 fault virtual address = 0x7572749f fault code = supervisor read, page not present instruction pointer = 0x20:0xc09c0551 stack pointer = 0x28:0xc43f6ab4 frame pointer = 0x28:0xc43f6adc code segment = base 0x0, limit 0xfffff, type 0x1b = DPL 0, pres 1, def32 1, gran 1 processor eflags = interrupt enabled, resume, IOPL = 0 current process = 0 (em0 taskq) #9 0xc0be731b in calltrap () at ../../../i386/i386/exception.s:165 #10 0xc09c0551 in in_lltable_lookup (llt=0xc4955200, flags=8192, l3addr=0xc43f6b60) at ../../../netinet/in.c:1361 #11 0xc09b8ea7 in arpintr (m=0xc48dcd00) at if_llatbl.h:202 #12 0xc096f899 in netisr_dispatch_src (proto=7, source=0, m=0xc48dcd00) at ../../../net/netisr.c:917 #13 0xc096fb30 in netisr_dispatch (proto=7, m=0xc48dcd00) at ../../../net/netisr.c:1004 #14 0xc0967c11 in ether_demux (ifp=0xc470b400, m=0xc48dcd00) at ../../../net/if_ethersubr.c:895 #15 0xc0968163 in ether_input (ifp=0xc470b400, m=0xc48dcd00) at ../../../net/if_ethersubr.c:754 #16 0xc063bcaa in em_rxeof (adapter=0xc470d000, count=99) at ../../../dev/e1000/if_em.c:4610 #17 0xc063dfc7 in em_handle_rxtx (context=0xc470d000, pending=1) at ../../../dev/e1000/if_em.c:1763 (kgdb) frame 10 #10 0xc09c0551 in in_lltable_lookup (llt=0xc4955200, flags=8192, l3addr=0xc43f6b60) at ../../../netinet/in.c:1361 1361 LIST_FOREACH(lle, lleh, lle_next) { (kgdb) list 1356 KASSERT(l3addr->sa_family == AF_INET, 1357 ("sin_family %d", l3addr->sa_family)); 1358 1359 hashkey = sin->sin_addr.s_addr; 1360 lleh = &llt->lle_head[LLATBL_HASH(hashkey, LLTBL_HASHMASK)]; 1361 LIST_FOREACH(lle, lleh, lle_next) { 1362 struct sockaddr_in *sa2 = (struct sockaddr_in *)L3_ADDR(lle); 1363 if (lle->la_flags & LLE_DELETED) 1364 continue; 1365 if (sa2->sin_addr.s_addr == sin->sin_addr.s_addr) (kgdb) print *llt $5 = {llt_link = {sle_next = 0x0}, lle_head = {{lh_first = 0xc760d580}, { lh_first = 0x0}, {lh_first = 0x0}, {lh_first = 0xc75c3900}, { lh_first = 0x0} , {lh_first = 0xc49a8380}, { lh_first = 0x0}, {lh_first = 0x0}, {lh_first = 0x0}, {lh_first = 0x0}, { lh_first = 0x0}, {lh_first = 0x0}, {lh_first = 0x0}, {lh_first = 0x0}, { lh_first = 0x0}, {lh_first = 0xc61be780}, {lh_first = 0x0}, { lh_first = 0x0}, {lh_first = 0x0}}, llt_af = 2, llt_ifp = 0xc470b400, llt_new = 0xc09bdd60 , llt_free = 0xc09c0490 , llt_prefix_free = 0xc09c09b0 , llt_lookup = 0xc09c0510 , llt_rtcheck = 0xc09bdbe0 , llt_dump = 0xc09bd9c0 } (kgdb) print *l3addr $6 = {sa_len = 16 '\020', sa_family = 2 '\002', sa_data = "\000\000??*\001\000\000\000\000\000\000\000"} (kgdb) print lle $7 = (struct llentry *) 0x75727473 (kgdb) print lleh $8 = (struct llentries *) 0xc4955210 (kgdb) print *lleh $9 = {lh_first = 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: > head/sys/netinet/if_ether.c > > Modified: head/sys/netinet/if_ether.c > ============================================================================== > --- head/sys/netinet/if_ether.c Tue Oct 20 17:50:36 2009 (r198300) > +++ head/sys/netinet/if_ether.c Tue Oct 20 17:55:42 2009 (r198301) > @@ -175,18 +175,18 @@ arptimer(void *arg) > CURVNET_SET(ifp->if_vnet); > IF_AFDATA_LOCK(ifp); > LLE_WLOCK(lle); > - if (((lle->la_flags & LLE_DELETED) || > - (time_second >= lle->la_expire)) && > - (!callout_pending(&lle->la_timer) && > + if ((!callout_pending(&lle->la_timer) && > callout_active(&lle->la_timer))) { > (void) llentry_free(lle); > ARPSTAT_INC(timeouts); > - } else { > - /* > - * Still valid, just drop our reference > - */ > - LLE_FREE_LOCKED(lle); > + } > +#ifdef DIAGNOSTICS > + else { > + struct sockaddr *l3addr = L3_ADDR(lle); > + log(LOG_INFO, "arptimer issue: %p, IPv4 address: \"%s\"\n", lle, > + inet_ntoa(((const struct sockaddr_in *)l3addr)->sin_addr)); > } > +#endif > IF_AFDATA_UNLOCK(ifp); > CURVNET_RESTORE(); > } > @@ -377,7 +377,7 @@ retry: > > if (renew) { > LLE_ADDREF(la); > - la->la_expire = time_second; > + la->la_expire = time_second + V_arpt_down; > callout_reset(&la->la_timer, hz * V_arpt_down, arptimer, la); > la->la_asked++; > LLE_WUNLOCK(la); >