From owner-freebsd-bugs@FreeBSD.ORG Sat Jan 17 16:20:04 2009 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4489B1065675 for ; Sat, 17 Jan 2009 16:20:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 1FB308FC16 for ; Sat, 17 Jan 2009 16:20:04 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n0HGK3Bi079792 for ; Sat, 17 Jan 2009 16:20:04 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n0HGK3Ta079790; Sat, 17 Jan 2009 16:20:03 GMT (envelope-from gnats) Resent-Date: Sat, 17 Jan 2009 16:20:03 GMT Resent-Message-Id: <200901171620.n0HGK3Ta079790@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Dmitrij Tejblum Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 54EE8106566B for ; Sat, 17 Jan 2009 16:13:04 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id 281C78FC21 for ; Sat, 17 Jan 2009 16:13:04 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id n0HGD3t7009449 for ; Sat, 17 Jan 2009 16:13:03 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id n0HGD3Qj009412; Sat, 17 Jan 2009 16:13:03 GMT (envelope-from nobody) Message-Id: <200901171613.n0HGD3Qj009412@www.freebsd.org> Date: Sat, 17 Jan 2009 16:13:03 GMT From: Dmitrij Tejblum To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: kern/130652: Possible deadlock in rt_check() (sys/net/route.c) X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Jan 2009 16:20:04 -0000 >Number: 130652 >Category: kern >Synopsis: Possible deadlock in rt_check() (sys/net/route.c) >Confidential: no >Severity: serious >Priority: high >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Jan 17 16:20:03 UTC 2009 >Closed-Date: >Last-Modified: >Originator: Dmitrij Tejblum >Release: 7.1-STABLE >Organization: OOO Yandex >Environment: FreeBSD 7.1-STABLE; net/route.c 1.120.2.7 >Description: Some excerpt from rt_check(): rt_check() { /*1*/ RT_LOCK(rt0); retry: ... if ((rt = rt0->rt_gwroute) != NULL) { /*2*/ RT_LOCK(rt); /* NB: gwroute */ .... } if (rt == NULL) { /* NOT AN ELSE CLAUSE */ /*3*/ RT_TEMP_UNLOCK(rt0); /* MUST return to undo this */ /*4*/ rt = rtalloc1_fib(rt0->rt_gateway, 1, 0UL, fibnum); .... /*5*/ RT_RELOCK(rt0); .... rt0->rt_gwroute = rt; } RT_LOCK_ASSERT(rt); RT_UNLOCK(rt0); .... } The function deals with route rt0 and rt. Usually, it locks rt0 in point /*1*/, then locks rt = rt0->rt_gwroute in point /*2*/, then unlock rt0 and done. But sometimes, in lock rt inside rtalloc1_fib() in point /*4*/. Then, in point /*5*/, it locks rt0, which was unlocked in point /*3*/. The order of locking of rt0 and rt is reversed, so a deadlock is possible. (Also, if after RT_RELOCK(rt0) we found that rt0 is unusable, we should not forget to free rt before retry.) >How-To-Repeat: >Fix: Patch attached with submission follows: --- net/route.c 2008-12-05 20:40:46.000000000 +0300 +++ net/route.c 2009-01-17 18:59:12.000000000 +0300 @@ -1634,24 +1634,31 @@ retry: * Relock it and lose the added reference. * All sorts of things could have happenned while we * had no lock on it, so check for them. + * rt need to be unlocked to avoid possible deadlock. */ + RT_UNLOCK(rt); RT_RELOCK(rt0); - if (rt0 == NULL || ((rt0->rt_flags & RTF_UP) == 0)) + if (rt0 == NULL || ((rt0->rt_flags & RTF_UP) == 0)) { /* Ru-roh.. what we had is no longer any good */ + RTFREE(rt); goto retry; + } /* * While we were away, someone replaced the gateway. * Since a reference count is involved we can't just * overwrite it. */ if (rt0->rt_gwroute) { - if (rt0->rt_gwroute != rt) { - RTFREE_LOCKED(rt); - goto retry; - } + if (rt0->rt_gwroute != rt) + RTFREE(rt); } else { rt0->rt_gwroute = rt; } + /* + * Since rt was not locked, we need recheck that + * it still may be used (e.g. up) + */ + goto retry; } RT_LOCK_ASSERT(rt); RT_UNLOCK(rt0); >Release-Note: >Audit-Trail: >Unformatted: