From owner-freebsd-stable@FreeBSD.ORG Tue Jun 21 10:51:59 2005 Return-Path: X-Original-To: freebsd-stable@FreeBSD.org Delivered-To: freebsd-stable@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2B1D016A41C; Tue, 21 Jun 2005 10:51:59 +0000 (GMT) (envelope-from glebius@FreeBSD.org) Received: from relay.bestcom.ru (relay.bestcom.ru [217.72.144.5]) by mx1.FreeBSD.org (Postfix) with ESMTP id 855A143D48; Tue, 21 Jun 2005 10:51:58 +0000 (GMT) (envelope-from glebius@FreeBSD.org) Received: from cell.sick.ru (root@cell.sick.ru [217.72.144.68]) by relay.bestcom.ru (8.13.1/8.12.9) with ESMTP id j5LApuNM033876 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Tue, 21 Jun 2005 14:51:57 +0400 (MSD) (envelope-from glebius@FreeBSD.org) Received: from cell.sick.ru (glebius@localhost [127.0.0.1]) by cell.sick.ru (8.13.1/8.12.8) with ESMTP id j5LApt1M036805 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 21 Jun 2005 14:51:55 +0400 (MSD) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.sick.ru (8.13.1/8.13.1/Submit) id j5LApseA036804; Tue, 21 Jun 2005 14:51:54 +0400 (MSD) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.sick.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Tue, 21 Jun 2005 14:51:54 +0400 From: Gleb Smirnoff To: Jeremie Le Hen Message-ID: <20050621105154.GA36538@cell.sick.ru> References: <20050621070427.GA738@obiwan.tataz.chchile.org> <20050621090701.GB34406@cell.sick.ru> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Nq2Wo0NMKNjxTN9z" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20050621090701.GB34406@cell.sick.ru> User-Agent: Mutt/1.5.6i X-Virus-Scanned: ClamAV version devel-20050125, clamav-milter version 0.80ff on relay.bestcom.ru X-Virus-Status: Clean Cc: qingli@FreeBSD.org, sam@FreeBSD.org, andre@FreeBSD.org, freebsd-stable@FreeBSD.org Subject: Re: panic in RELENG_5 UMA X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Jun 2005 10:51:59 -0000 --Nq2Wo0NMKNjxTN9z Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: 8bit [ cc'ing parties involved in this part of code] On Tue, Jun 21, 2005 at 01:07:01PM +0400, Gleb Smirnoff wrote: T> On Tue, Jun 21, 2005 at 09:04:27AM +0200, Jeremie Le Hen wrote: T> J> #25 0xc05a0a0b in m_freem (mb=0x0) at uma.h:304 T> J> No locals. T> J> #26 0xc05ee0d5 in arpresolve (ifp=0xc1a5b000, rt0=0xc1d44000, m=0xc1be7200, T> J> dst=0xd6d3fa94, desten=0xd6d3fa2c "/??]??????w??") T> J> at ../../../netinet/if_ether.c:442 T> J> la = (struct llinfo_arp *) 0xc1a75a00 T> J> sdl = (struct sockaddr_dl *) 0xc2128910 T> J> error = -1038972656 T> J> rt = (struct rtentry *) 0xc1d44000 T> T> IMHO, this looks like a race. The route is not locked, when T> its llinfo is edited. T> T> Probably the mbuf was freed when arp reply arrived and la_hold was send. T> Look into in_arpinput() near 736: T> T> (*ifp->if_output)(ifp, la->la_hold, rt_key(rt), rt); T> la->la_hold = 0; T> T> Yeah, I have just triggered another panic running 15 instances of this script on T> SMP box: T> T> ( T> while (true); do T> arp -d 81.19.64.111 >/dev/null 2>&1; T> ping -c 1 -t 1 81.19.64.111 >/dev/null 2>&1; T> done T> ) & T> T> But my duplicate free is in fxp_txeof(). This means that output thread has T> won the race. I suppose that the attached patch closes your race. However, there is still race between RTM_DELETE and output path. The above script still drops kernel to panic, but the other one. Output path works with already freed llinfo: #28 0xc0507000 in m_freem (mb=0x0) at mbuf.h:410 #29 0xc053fde3 in arpresolve (ifp=0xc2012800, rt0=0xc22fcdec, m=0xc25a8000, dst=0xe720bb28, desten=0xe720bacc "uøbÀ+\001") at /usr/src/sys/netinet/if_ether.c:443 #30 0xc0538078 in ether_output (ifp=0xc2012800, m=0xc25a8000, dst=0xe720bb28, rt0=0xc22fcdec) at /usr/src/sys/net/if_ethersubr.c:173 #31 0xc054b5b4 in ip_output (m=0xc25a8000, opt=0xc25a80ac, ro=0xe720bb24, flags=0x20, imo=0x0, inp=0xc25eb5a0) at /usr/src/sys/netinet/ip_output.c:772 #32 0xc054d36b in rip_output (m=0xc25a8000, so=0x0, dst=0x0) at /usr/src/sys/netinet/raw_ip.c:320 #33 0xc054de7b in rip_send (so=0xc248c914, flags=0x0, m=0xc25a8000, nam=0xc218d410, control=0x0, td=0xc224d7d0) at /usr/src/sys/netinet/raw_ip.c:785 #34 0xc050a30f in sosend (so=0xc248c914, addr=0xc218d410, uio=0xe720bc3c, top=0xc25a8000, control=0x0, flags=0x0, td=0xc224d7d0) at /usr/src/sys/kern/uipc_socket.c:827 (kgdb) frame 29 #29 0xc053fde3 in arpresolve (ifp=0xc2012800, rt0=0xc22fcdec, m=0xc25a8000, dst=0xe720bb28, desten=0xe720bacc "uøbÀ+\001") at /usr/src/sys/netinet/if_ether.c:443 443 m_freem(la->la_hold); (kgdb) p *la $3 = { la_le = { le_next = 0xdeadc0de, le_prev = 0xdeadc0de }, la_rt = 0xdeadc0de, la_hold = 0xdeadc0de, la_preempt = 0xc0de, la_asked = 0xdead } Fixing this one is harder. We take la from unlocked rtentry obtained via rt_check(), or from arplookup(). The latter drops lock on rtentry, too. Then we do some work and use this la. It may have already been freed in arp_rtrequest(), the RTM_DELETE case. I see two approaches here: 1) Protecting llinfo with route lock. In this case we need rt_check() to return locked *rt (just reference won't help). We also need arplookup() to return locked rt. And do not unlock it withing all arpresolve() and a big part of in_arpinput() functions. 2) Add mutex to llinfo_arp. I'm afraid this will hurt performance. -- Totus tuus, Glebius. GLEBIUS-RIPN GLEB-RIPE --Nq2Wo0NMKNjxTN9z Content-Type: text/plain; charset=koi8-r Content-Disposition: attachment; filename="if_ether.c.diff" Index: if_ether.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/if_ether.c,v retrieving revision 1.137 diff -u -r1.137 if_ether.c --- if_ether.c 5 Jun 2005 03:13:12 -0000 1.137 +++ if_ether.c 21 Jun 2005 10:36:08 -0000 @@ -438,11 +438,11 @@ * response yet. Replace the held mbuf with this * latest one. */ + RT_LOCK(rt); if (la->la_hold) m_freem(la->la_hold); la->la_hold = m; if (rt->rt_expire) { - RT_LOCK(rt); rt->rt_flags &= ~RTF_REJECT; if (la->la_asked == 0 || rt->rt_expire != time_second) { rt->rt_expire = time_second; @@ -459,8 +459,8 @@ } } - RT_UNLOCK(rt); } + RT_UNLOCK(rt); return (EWOULDBLOCK); } @@ -642,6 +642,8 @@ goto reply; la = arplookup(isaddr.s_addr, itaddr.s_addr == myaddr.s_addr, 0); if (la && (rt = la->la_rt) && (sdl = SDL(rt->rt_gateway))) { + struct mbuf *hold; + /* the following is not an error when doing bridging */ if (!bridged && rt->rt_ifp != ifp #ifdef DEV_CARP @@ -729,11 +731,13 @@ if (rt->rt_expire) rt->rt_expire = time_second + arpt_keep; rt->rt_flags &= ~RTF_REJECT; - RT_UNLOCK(rt); la->la_asked = 0; la->la_preempt = arp_maxtries; - if (la->la_hold) { - (*ifp->if_output)(ifp, la->la_hold, rt_key(rt), rt); + hold = la->la_hold; + la->la_hold = 0; + RT_UNLOCK(rt); + if (hold) { + (*ifp->if_output)(ifp, hold, rt_key(rt), rt); la->la_hold = 0; } } --Nq2Wo0NMKNjxTN9z--